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

Add sidekiq 7.2 to test matrix #22

Merged
merged 3 commits into from
Feb 7, 2024
Merged

Add sidekiq 7.2 to test matrix #22

merged 3 commits into from
Feb 7, 2024

Conversation

rwojsznis
Copy link
Owner

Initial discovery on #21

Root cause: https://github.com/sidekiq/sidekiq/blob/main/Changes.md#720

Adjust redis-client adapter to avoid method_missing [#6083] This can result in app code breaking if your app's Redis API usage was depending on Sidekiq's adapter to correct invalid redis-client API usage

7.2 changed their internal API regarding calling redis-client; Support to 7.0 was added by community in #18 and I have very vague memory of it

@rwojsznis rwojsznis self-assigned this Dec 13, 2023
@rwojsznis
Copy link
Owner Author

In case anyone is waiting for this - not forgotten, just got busy with everyday work; will try to keep this gem alive :)

@9mm
Copy link

9mm commented Jan 12, 2024

Awesome, thanks

@9mm
Copy link

9mm commented Jan 12, 2024

I def use this gem on like every project btw, so you know your work is awesome. It solves a really important need that for some reason isnt just built into sidekiq.


# very not fancy (https://78.media.tumblr.com/tumblr_lzkpw7DAl21qhy6c9o2_400.gif)
# solution, but should do the job
Open3.popen3(cmd) do |stdin, stdout, stderr, thread|
begin
Timeout.timeout(5) do
Timeout.timeout(3) do
Copy link
Owner Author

Choose a reason for hiding this comment

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

still no better idea for this one, leaving as it

@@ -1,3 +1,4 @@
$LOAD_PATH.unshift(File.expand_path('../lib', __dir__))
Copy link
Owner Author

Choose a reason for hiding this comment

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

not sure how this worked before; seems like something changed with load paths I'm not aware of? require stopped working 🤔 will not investigate if the build is green tho 🙈

Comment on lines -22 to -26
spec.add_development_dependency "bundler"
spec.add_development_dependency "rake"
spec.add_development_dependency "mocha", "~> 0.14.0"
spec.add_development_dependency "minitest"
spec.add_development_dependency "appraisal"
Copy link
Owner Author

Choose a reason for hiding this comment

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

unsure what's the best practice nowadays, seems like sidekiq itself defines all this stuff in Gemfile now

@rwojsznis rwojsznis marked this pull request as ready for review February 7, 2024 18:29
@rwojsznis
Copy link
Owner Author

All tests green; will release new version this week; dropped sidekiq 5 from the matrix as it seems EOLed?

@rwojsznis rwojsznis merged commit 3db1099 into main Feb 7, 2024
18 checks passed
@rwojsznis rwojsznis deleted the sidekiq-7.2-support branch February 7, 2024 18:33
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.

2 participants