-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unique and expiring jobs (Enterprise Faktory) #45
Unique and expiring jobs (Enterprise Faktory) #45
Conversation
Codecov ReportAttention:
Additional details and impacted files
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks for taking it on!
Only a few smaller notes.
assert_eq!(msg, "Job not unique"); | ||
} else { | ||
panic!("Expected protocol error.") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also check that both job1
and job2
were enqueued?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be addressed in PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good now, thank you!
One thing I'd recommend is to make use of the doc_cfg
nightly feature specifically on docs.rs to highlight things that are only available with ent
. You can see an example of how to do this in fantoccini below. We can do that in a separate PR though!
https://github.com/jonhoo/fantoccini/blob/b99209b67b176a617751885ca241189ea9c9940c/src/lib.rs#L143
https://github.com/jonhoo/fantoccini/blob/b99209b67b176a617751885ca241189ea9c9940c/src/lib.rs#L179
Released in 0.12.3 🎉 |
Putting those features under the feature called "ent", because looks like it is already a common abbreviation/label for Enterprise Faktory.
Referencing the Faktory's wiki with regard to API.
Covering these Ent Faktory features:
Will be added as separate PRs:
This change is