Fix #499: add use_namespace_prefix_keys option to enable message keys that have unhashed namespace prefix#842
Open
JoaoAlmeida20 wants to merge 3 commits intoBogdanp:masterfrom
Open
Conversation
…ge keys that keep the namespace prefix unhashed
Collaborator
|
Hi @JoaoAlmeida20, skipping the conversation is ok since there is the existing issue/PR/feedback, I'll have a look, thanks |
| *, | ||
| namespace: str = "dramatiq-results", | ||
| encoder: typing.Optional[Encoder] = None, | ||
| use_namespace_prefix_keys: bool = False, |
Collaborator
There was a problem hiding this comment.
The new parameter needs to be added to __init__ of the RedisBackend and MemcachedBackend classes that subclass this.
dramatiq/results/backend.py
Outdated
| "actor_name": message.actor_name, | ||
| "message_id": message.message_id, | ||
| } | ||
| hashed_key = hashlib.md5(message_key.encode("utf-8")).hexdigest() |
Collaborator
There was a problem hiding this comment.
Is there a reason to still md5 the part after the namespace? #615 Didn't do that.
Feels like it would be nicer to not use any MD5? That way you have different useful prefixes to work with in the final key:
namespace:
namespace:queue:
namespace:queue:actor:
Author
…none of the key components are md5 hashed, add use_namespace_prefix_keys to RedisBackend and MemcachedBackend inits
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #499, based on #615 PR, which was closed for being a breaking change, making it necessary to guard it behind a flag or otherwise guaranteeing compatibility with old style keys, which didn't end up being addressed at the time. Here, the new behavior only happens if we set use_namespace_prefix_keys to True in the ResultsBackend instance.
I didn't start a discussion since the PR is based on an existing PR and the feedback it had already gotten, apologies if that was skipping a step.