chaps-io / gush

Fast and distributed workflow runner using ActiveJob and Redis
MIT License
1.03k stars 103 forks source link

Allow configuring redis via a option hash #102

Closed wnm closed 3 weeks ago

wnm commented 1 year ago

Redis 6+ requires SSL to connect. Heroku does not use SSL, they use HTTP routing instead.

This PR allows to configure Redis via an option hash like:

# config/initializers/gush.rb
Gush.configure do |config|
  config.redis = { url: "redis://localhost:6379", ssl_params: { verify_mode: OpenSSL::SSL::VERIFY_NONE }}
end
pokonski commented 1 year ago

Thank you for this! A good addition regardless of changes in Rails :)

wnm commented 1 year ago

glad you like it @pokonski, for what its worth we are running the fork with the code in this PR in production right now and it works as expected. Would love to get back to using the upstream version though, instead of using our own fork. Let me know if you need anything in order to merge this in!?

pokonski commented 1 year ago

That is indeed great to know it runs well, I just need a bit time to release it, but will definitely do soon!

denisstael commented 8 months ago

Some time ago, we also needed to slightly modify the settings for Gush in order to pass additional options to the Redis initialization.

In our case, Redis access was password-protected, and we added this option in a fork of the repository. Today, we also have projects running in production with this modified version.

I was thinking of contributing to the project by suggesting adding this option, then I saw this pull request that allows passing many options to Redis, and I realized that it would also solve the password case.

It would be great to have this in the official version. I would suggest keeping the option to inform only the hash instead of also having the url option, to keep the Redis configuration centralized.

Furthermore, congratulations and thank you for the project, it's a real lifesaver for asynchronous executions, and thanks to @wnm for taking the initiative to open this PR!

wnm commented 3 months ago

@pokonski any updates on this 😇

wnm commented 3 weeks ago

just one more friendly push if we can merge this? would be nice to eventually land upstream, and not use our own fork 😊

pokonski commented 3 weeks ago

Hey @wnm, we now have #108. Will that be enough? It's available in 3.0

wnm commented 3 weeks ago

Ah fantastic, that sure is enough!! thanks so much 🙌

pokonski commented 3 weeks ago

And thanks again for the change, sorry about the duplication - it completely slipped my mind that we already had your PR for this!

wnm commented 3 weeks ago

@pokonski hehe no worries 😊

I just installed version 3.0.0 but I'm getting a NoMethodError: undefined method redis= for #<Gush::Configuration:, and when I run bundle open gush and inspect the code, I don't see the changes from #108. I can see the PR is merged, but I don't think it landed in 3.0.0?