search

jondot / sneakers

A fast background processing framework for Ruby and RabbitMQ
https://github.com/jondot/sneakers
MIT License
2.24k stars 333 forks source link

passing worker to the middleware call #414

Closed redwrx closed 2 years ago

redwrx commented 4 years ago

a minor change to pass the worker object to the middleware call https://github.com/jondot/sneakers/issues/415

mikker commented 4 years ago

We'd love to have this merged as it would make the Sneakers jobs look much better in Elastic APM (see https://github.com/elastic/apm-agent-ruby/pull/676#issue-360412848) ➕!

jondot commented 4 years ago

Thanks, looks like a great idea. would this break compatibility with existing middleware? (if so, is there a way to make it backwards compatible?)

mikker commented 4 years ago

Seems to me this would be breaking as the position of all arguments get shifted. Without knowing much about the rest of the codebase, one way to not be breaking would be to add self to one of the currently passed arguments, eg metadata[:worker] = self.

jondot commented 4 years ago

Interesting. I would also accept having two different signatures, one with this and one without (older one). Would you be willing to submit a PR or augment this one to be backward compatible? if so I'd be happy to merge of course

redwrx commented 4 years ago

Interesting. I would also accept having two different signatures, one with this and one without (older one). Would you be willing to submit a PR or augment this one to be backward compatible? if so I'd be happy to merge of course

I agree and will update this PR to follow @mikker approach

redwrx commented 4 years ago

Interesting. I would also accept having two different signatures, one with this and one without (older one). Would you be willing to submit a PR or augment this one to be backward compatible? if so I'd be happy to merge of course

I pushed the code that uses metadata to pass around the worker name. It passes all tests for all ruby versions, except Ruby-head. Seems like there is a bug with minitest that prevents the tests to run successfully ArgumentError: wrong number of arguments (given 2, expected 1) @jondot

https://github.com/seattlerb/minitest/issues/689