Skip to content
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

Allow defining a job priority [implements #56] #57

Closed
wants to merge 3 commits into from

Conversation

KushalP
Copy link
Contributor

@KushalP KushalP commented Mar 25, 2019

This is a work-in-progress PR.

Tasks

  • Add priority column
  • Update rihanna_jobs_enqueued_at_id index to use priority column
  • Define a priority method on Rihanna.Job
  • Sufficient testing of the behaviour

Testing

What's the preferred way to test this behaviour? I'm a little confused with the style taken. If you could provide a single sample test case, that's probably sufficient here.

@KushalP
Copy link
Contributor Author

KushalP commented Mar 25, 2019

Just realised the description is a little vague here. What's the preferred place for me to put any tests related to this new new field? How many tests are sufficient? Is setting a default value okay in this case?

@KushalP KushalP force-pushed the add-job-priority branch 3 times, most recently from 4a99217 to 7a7d902 Compare March 25, 2019 16:10
Defining a default priority
---------------------------

It makes sense to provide a default priority where possible. This way
the implementation is backwards compatible for any users who are
implementing their own jobs with `Rihanna.Job`.

The lowest priority is the default, allowing developers to increase
the priority of their jobs as/when they need to.

The default priority value
--------------------------

A default of 19 has been chosen to conform with the Unix process
priority scale[1]

[1] https://en.wikipedia.org/wiki/Nice_(Unix)
@KushalP KushalP force-pushed the add-job-priority branch 6 times, most recently from 3119635 to 7d695c3 Compare March 25, 2019 16:40
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.

None yet

1 participant