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

flushall and flushdb vs namespaces #56

Open
yaauie opened this issue Apr 10, 2013 · 12 comments
Open

flushall and flushdb vs namespaces #56

yaauie opened this issue Apr 10, 2013 · 12 comments

Comments

@yaauie
Copy link
Member

yaauie commented Apr 10, 2013

Since these are especially destructive (beyond the scope of the namespace) and rather surprising to anyone who would think that namespacing provides segregation, perhaps #flushall and #flushdb should be blocked in a future version, requiring the user to use RedisNamespace#redis directly.

@steveklabnik
Copy link
Member

Yeah. This will cause lots of havok in the Resque tests....

@carlzulauf
Copy link
Contributor

👍

@yaauie
Copy link
Member Author

yaauie commented Apr 10, 2013

@steveklabnik definitely in a major version release if at all. The trouble is that the documentation claims that this wrapper "Namespaces all Redis calls", and yet it doesn't. Same for #34.

The actual behaviour is very surprising in a very destructive way, so either that needs to stop happening or the documentation needs to be updated to something like "Namespaces most Redis calls that we know and care about, but lets the rest pass through unchanged" 😄.

@plukevdh
Copy link

plukevdh commented Sep 9, 2013

Does this mean that there is no way to flush a namespace?

@yaauie
Copy link
Member Author

yaauie commented Sep 10, 2013

@plukevdh yes, that is correct.

@dmitry
Copy link

dmitry commented Mar 4, 2015

We are doing it currently like redis.del(redis.keys('*')) in our tests (or previously redis.keys('*').each { |key| redis.del(key) }). And I guess there are no better way of doing the same thing.

👍 for #flushall and #flushdb to raise an error and use redis directly instead of current behaviour.

@yaauie
Copy link
Member Author

yaauie commented Mar 4, 2015

@dmitry if your redis-server supports it (versions >= 2.7.105), scan is a lot safer (and the ruby adapter has scan_each which behaves a lot like keys)

@dmitry
Copy link

dmitry commented Mar 4, 2015

@yaauie thanks for the suggestion. We have parallel tests that are isolated by the redis-namespace and they are running in sequential flow, so the keys + del is enough for us for this task.

@Fryie
Copy link

Fryie commented Mar 6, 2015

Thanks @dmitry. I believe you also have to wrap that call, since del fails on an empty array:

if redis.keys.any?
  redis.del(redis.keys)
end

@dmitry
Copy link

dmitry commented Mar 6, 2015

@Fryie yes, that's right. To prevent from 2 calls, you have to use:

if (keys = redis.keys) && !keys.empty?
  redis.del(keys)
end

@richhollis
Copy link

Would there be any value in adding a new method that just deletes the namespaced keys? I could submit a PR if there is.

@fatkodima
Copy link
Contributor

So seems like this issue can be closed, because using them already produces a warning and will raise in 2.0 -

ADMINISTRATIVE_COMMANDS.keys.each do |command|
define_method(command) do |*args, &block|
raise NoMethodError if deprecations?
if warning?
warn("Passing '#{command}' 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 #{call_site})"
)
end
call_with_namespace(command, *args, &block)
end
ruby2_keywords(command) if respond_to?(:ruby2_keywords, true)
end

Would there be any value in adding a new method that just deletes the namespaced keys? I could submit a PR if there is.

Makes sense to me.

francois-ferrandis added a commit to betagouv/rdv-service-public that referenced this issue Feb 28, 2024
francois-ferrandis added a commit to betagouv/rdv-service-public that referenced this issue Mar 4, 2024
* Vider Redis entre chaque spec

* Use redis-namespace gem to handle parallel_tests

* Define namespace in all envs to easily find app writes

* See resque/redis-namespace#56

* Use a Redis set instead of a list

https://redis.io/docs/data-types/sets/

Could not use command LPOS right now because redis-namespace
does not support it. Besides, using a set is appropriate here.

* Clarify key name

* Add comment for new gem
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

No branches or pull requests

8 participants