search

seomoz / qless

Queue / Pipeline Management
MIT License
294 stars 76 forks source link

Remove uses of metriks and its dependencies. #283

Closed sistawendy closed 4 years ago

sistawendy commented 4 years ago

The atomic gem, which is a dependency of metriks, has started breaking builds in Ruby 2.4.1. This pull request removes the only use of metriks and its dependencies in this repo, which is an optional middleware.

phil-hildebrand commented 4 years ago

Actually, I think there will need to be some changes if metriks is removed.

When a job runs, it appears to call around_perform which currently expects Metriks

Did the tests all pass with your change?

phil-hildebrand commented 4 years ago

Maybe you could just take out the require and stub the metriks functions so that middleware still works, it just doen't time the jobs?

phil-hildebrand commented 4 years ago

@b4hand / @danlecocq would either of you be available to weigh in and/or make suggestions?

sistawendy commented 4 years ago

If you don't tell Rack to use the middleware, as Barbosa doesn't, the middleware never gets called. Neutering the middleware is an option. Did I mention that we have this branch deployed to production right now?

The tests, by the way, are a disaster.

phil-hildebrand commented 4 years ago

Yeah - that make sense. I do think for those that may use it (middle ware) outside of moz, it might make sense just to stub it out so that if they happen not to have the version pinned, it doesn't blow up.

I hadn't tried to run the tests yet, so if there are a bunch failing already, I'd agree barbosa in prod is a valid test case 👍

sistawendy commented 4 years ago

Hollowed-out metriks middleware added.