Skip to content

Conversation

@drmikecrowe
Copy link

Two new features:

  1. Metadata: This is additional data added to the job, but not part of attrs (so does not affect remove function)
  2. Replace: This replaces a future job with updated info (requires the meta data feature)

…arch)

Added replace function to update future job
package.json Outdated
{
"name": "yajob",
"version": "2.1.2",
"version": "2.1.3",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not update version in PR please. This changes likely to be a minor update.

@floatdrop
Copy link
Owner

Why just not store meta in attr like this:

mails.put({
    from: 'floatdrop@gmail.com',
    to: 'nodejs-dev@dev-null.com',
    meta: {body: 'You have 1 new notification'}
}, {
    schedule: d
});

Remove functionality will not be affected (or I misunderstanding something).

@drmikecrowe
Copy link
Author

drmikecrowe commented Jun 14, 2016

Well, maybe I didn't understand the remove fully. Consider your example, and this:

mails.remove({
    from: 'floatdrop@gmail.com',
    to: 'nodejs-dev@dev-null.com'
});

Would this remove the entry without specifying the meta parameter? If so, you are right, I don't need the meta.

The whole point of the PR was to allow the following:

  1. Change data in a future job (the replace function)
  2. Provide a way to search by args without certain data (in this case, search update "body" in your email by only specifying "from"/"to" in the search parameters

(I'll close this PR and submit just the replace if this is the case)

@drmikecrowe
Copy link
Author

Right, that doesn't work. Consider this test (that fails):

test('removes job with partial attr match', async t => {
    const queueDb = await new QueueDb();
    const queue = yajob(queueDb.uri);

    try {
        await queue.put({test: 'wow', param: 1});
        await queue.remove({test: 'wow'});
        const jobs = await queueDb.db.collection('default').find().toArray();
        t.is(jobs.length, 0, 'should remove job from queue');
    } finally {
        await queueDb.close();
    }
});

Let me clean up my PR and see what you think

@floatdrop
Copy link
Owner

floatdrop commented Jun 17, 2016

@drmikecrowe sorry for late response – I am quite sure, that right way is to add option strict: false to remove method. This way will keep us from meta field and additional method replace.

@drmikecrowe
Copy link
Author

I tried to get the remove function to work with the meta Field in the
attrs, but could not get the query to work properly.

The only way I was able to get it to work, was to have the metadata in a
separate part of the object.

What I've committed now is actually solid and I'm starting to use it in my
project.
On Jun 17, 2016 2:01 AM, "Vsevolod Strukchinsky" notifications@github.com
wrote:

@drmikecrowe https://github.com/drmikecrowe sorry for late response – I
am thinking, that right way is to add option strict: false to remove
method. This way will keep us from meta field and additional method
replace.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAFgyB825tq9Y0JQPWqUT2zBd-jfeLnOks5qMjhDgaJpZM4I08g2
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants