Skip to content

Conversation

@james-lawrence
Copy link

clean up enqueue option generation to make it less error prone, and easier to add new fields.

bkirz and others added 30 commits March 10, 2014 14:20
Improve RetryExceptions middleware to support exception-dependent backoffs
This ensures that you’ll get an error if the job has
already been cancelled, rather than resurrecting it.
Copy link

Choose a reason for hiding this comment

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

Maybe it'd be worth DRYing this (this assumes that JSON.generate will handle primitives properly).

DEFAULT_OPTS = {
  'priority' => 0,
  'tags', => [],
  # etc
}

def self.build_opts_array(opts)
  DEFAULT_OPTS.reduce([opts.fetch(:delay, 0)]) do |result, (key, default)|
    value = JSON.generate(opts.fetch(key.to_sym, default))
    result << [key, value]
  end
end

It'd be great if you could just slice out what you're interested in from the opts Hash and apply defaults for the missing values.

Copy link
Author

Choose a reason for hiding this comment

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

agreed a little unsure of it atm. had to stop work mid way through the branch =) picking up the work again today.

Copy link
Author

Choose a reason for hiding this comment

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

doesn't work with base types so sad =(

Copy link

Choose a reason for hiding this comment

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

YMMV, but the benefit of Hash#fetch comes in when you expect the value in the Hash to be false or nil sometimes, otherwise, might as well do opts[:depends] || []

@jzaleski, I miss you 💋

@tdg5
Copy link

tdg5 commented May 11, 2015

LGTM

james-lawrence added a commit that referenced this pull request May 12, 2015
@james-lawrence james-lawrence merged commit 9deb039 into master May 12, 2015
@james-lawrence james-lawrence deleted the fix-requeue branch May 12, 2015 12:30
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.

7 participants