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

redis-namespace asks to use admistrative commands directly #578

Closed
Wolfer opened this issue Mar 2, 2021 · 8 comments · Fixed by #581
Closed

redis-namespace asks to use admistrative commands directly #578

Wolfer opened this issue Mar 2, 2021 · 8 comments · Fixed by #581

Comments

@Wolfer
Copy link
Contributor

Wolfer commented Mar 2, 2021

When Sidekiq configured use redis namespaces like this

opts = {
  host:      APP_CONFIG[:redis_host].to_s,
  namespace: REDIS_NAMESPACE
}

Sidekiq.configure_server do |config|
  config.redis = opts
end

Sidekiq.configure_client do |config|
  config.redis = opts
end

Deprecated text appears at startup

Passing 'info' command to redis as is; administrative commands cannot be effectively namespaced and should be called on the redis connection directly; passthrough has been deprecated and will be removed in redis-namespace 2.0 (at /usr/local/rvm/gems/ruby-2.6.3/gems/sidekiq-unique-jobs-7.0.4/lib/sidekiq_unique_jobs/sidekiq_unique_jobs.rb:191:in `block in fetch_redis_version')
@Wolfer
Copy link
Contributor Author

Wolfer commented Mar 2, 2021

@mhenrixon
Copy link
Owner

I don't really have a solution to that problem @Wolfer.

For now, I've followed the Sidekiq approach of recommending against namespaces.

Possibly, I could provide some type of administrative connection for this that the user configure.

Not sure why they decided to prevent administrative commands. They created a lot of friction with that decision.

Doesn't seem fair that I as a gem owner should have to provide extra configuration for a user of my gem to be able to fetch the redis version from the server itself.

I'll have to do some thinking on this topic. I'm hesitant to create custom stuff for redis namespace.

@mhenrixon
Copy link
Owner

You are aware it is considered bad practice with redis namespace right?

I'm not judging, maybe you have a project where that is outside of your control at the moment.

I thought it worth mentioning at least.

@Wolfer
Copy link
Contributor Author

Wolfer commented Mar 18, 2021

Yes, i know, but specific of project forces them to be used =(

@Wolfer
Copy link
Contributor Author

Wolfer commented Mar 18, 2021

I found than Sidekiq provide method redis_info which incapsulate getting redis info correctly. Please, review my PR #581

mhenrixon added a commit that referenced this issue Mar 18, 2021
@mhenrixon
Copy link
Owner

Released as v7.0.5

@Wolfer
Copy link
Contributor Author

Wolfer commented Mar 18, 2021

thank you =)
I make same PR in other your gem brpoplpush/brpoplpush-redis_script#20. Please, review its too

@mhenrixon
Copy link
Owner

Version v7.0.6 was just released with an updated dependency @Wolfer

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 a pull request may close this issue.

2 participants