arturictus / sidekiq_alive

Liveness probe for Sidekiq in Kubernetes deployments
MIT License
188 stars 57 forks source link

Allow Rack 3.x #101

Closed n-rodriguez closed 8 months ago

n-rodriguez commented 10 months ago

Thank you!

n-rodriguez commented 10 months ago

Hi there! Any news?

andrcuns commented 10 months ago

See this comment: https://github.com/arturictus/sidekiq_alive/pull/75#issuecomment-1363308683

I think if we release this, we need to make a major release. Rails below version 7.1 (which is just 2 weeks old), had a version restriction for rack: ~> 2.0, >= 2.2.4.

While technically it's not a breaking change, all folks with not a super up to date version of rails would not be able to update this gem.

@arturictus What do You think, should this be updated now that there is finally a rails version that does allow latest rack version?

andrcuns commented 10 months ago

It is also more involved than just bumping version restriction. Some components got extracted to separate gem, so it will require code changes and additional runtime dependencies

andrcuns commented 10 months ago

We could also use an opportunity to make new major release together with dropping ruby 2.7 which has been EOL for a bit now

n-rodriguez commented 9 months ago

I'm not sure this is really necessary. Sidekiq itself depends on rack and it works with both rack 2.x and rack 3.x.

andrcuns commented 9 months ago

Stumbled across this post from sidekiq author himself: https://www.mikeperham.com/2023/09/11/ruby-http-server-from-scratch/

Maybe worth a try given it implements exactly the usecase sidekiq-alive handles

arturictus commented 9 months ago

Stumbled across this post from sidekiq author himself: https://www.mikeperham.com/2023/09/11/ruby-http-server-from-scratch/

Maybe worth a try given it implements exactly the usecase sidekiq-alive handles

That's a great solution! thanks @andrcuns

n-rodriguez commented 8 months ago

Actually the solution seems to be simpler than that : just depend on rackup instead of rack like https://github.com/redis-store/redis-rack/commit/db8b382b2afd1fc526507e98f63aa40359734ce9

n-rodriguez commented 8 months ago

Stumbled across this post from sidekiq author himself: https://www.mikeperham.com/2023/09/11/ruby-http-server-from-scratch/

Maybe worth a try given it implements exactly the usecase sidekiq-alive handles

Actually it's not for everyone : "This functionality is now available in Sidekiq Enterprise 7.1.2."

And sidekiq itself still uses rack https://github.com/sidekiq/sidekiq/blob/main/sidekiq.gemspec#L28 for its WebUI

n-rodriguez commented 8 months ago

With this the CI is green and my app is on Rack 3 :) (and it should not break anything)

andrcuns commented 8 months ago

just depend on rackup

It would work but I personally don't think having open ended runtime dependency is a good idea and to still be compatible with sidekiq < 7, rackup has to be limited to < 2. I personally would prefer to get rid of directly depending on rack or rackup entirely and for the most part I already implemented a solution. I just need to make sure no weird issues pop up in real life usage.

Actually it's not for everyone : "This functionality is now available in Sidekiq Enterprise 7.1.2."

This isn't what I meant exactly. The way it is implemented in the Sidekiq Enterprise is to use a very basic http server that has no dependencies and I thought we could do the same in this gem. That is what I'm currently doing in: https://github.com/arturictus/sidekiq_alive/pull/106

n-rodriguez commented 8 months ago

That is what I'm currently doing in: #106

Good luck!