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

Allow configuring redis via a option hash #102

Closed
wants to merge 2 commits into from
Closed

Conversation

wnm
Copy link

@wnm wnm commented May 4, 2023

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
Copy link
Contributor

pokonski commented May 4, 2023

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

@wnm
Copy link
Author

wnm commented Jun 1, 2023

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
Copy link
Contributor

pokonski commented Jun 1, 2023

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

@denisstael
Copy link

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
Copy link
Author

wnm commented Mar 22, 2024

@pokonski any updates on this 😇

Pull changes from upstream
@wnm
Copy link
Author

wnm commented Jun 11, 2024

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

@pokonski
Copy link
Contributor

pokonski commented Jun 11, 2024

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

@wnm
Copy link
Author

wnm commented Jun 11, 2024

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

@wnm wnm closed this Jun 11, 2024
@pokonski
Copy link
Contributor

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

@wnm
Copy link
Author

wnm commented Jun 12, 2024

@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?

@wnm wnm mentioned this pull request Jun 18, 2024
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.

None yet

3 participants