Fix #499: add use_namespace_prefix_keys option to enable message keys that have unhashed namespace prefix#842
Conversation
…ge keys that keep the namespace prefix unhashed
|
Hi @JoaoAlmeida20, skipping the conversation is ok since there is the existing issue/PR/feedback, I'll have a look, thanks |
| "actor_name": message.actor_name, | ||
| "message_id": message.message_id, | ||
| } | ||
| hashed_key = hashlib.md5(message_key.encode("utf-8")).hexdigest() |
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:
…none of the key components are md5 hashed, add use_namespace_prefix_keys to RedisBackend and MemcachedBackend inits
themavik
left a comment
themavik
left a comment
There was a problem hiding this comment.
Default-off use_namespace_prefix_keys keeps MD5 keys; StubBackend test proves readable keys when enabled. nit: prefixed keys join queue, actor, and id with colons, so queue or actor names containing colons break naive split-based tooling or scripts.
LincolnPuzey
left a comment
LincolnPuzey
left a comment
There was a problem hiding this comment.
Looks good @JoaoAlmeida20 , can you resolve the conflict? thanks
3 participants
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.