Added support for _proxy-filtered flag.#366
Conversation
| #[derive(Debug, Deserialize)] | ||
| pub enum RedisEnterpriseCommandFlags { | ||
| /// A special enterprise only flag, make sure the commands marked with this flag will not be expose to | ||
| /// user via `command` command or on slow log. | ||
| ProxyFiltered, | ||
| } | ||
|
|
||
| impl From<&RedisEnterpriseCommandFlags> for &'static str { | ||
| fn from(value: &RedisEnterpriseCommandFlags) -> Self { | ||
| match value { | ||
| RedisEnterpriseCommandFlags::ProxyFiltered => "_proxy-filtered", | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
What do you think about using the bitflags here? P.S. If you decide to use the bitflags, the name should ideally be renamed to avoid the plural form, so the last s should be removed.
There was a problem hiding this comment.
I follow the none enterprise flags avoid, I do not see what we get by using bitflag, this enum is just for parsing the input of the proc macro.
There was a problem hiding this comment.
So, an object of the type RedisEnterpriseCommandFlags may only have one at a time? Even more so, the reason to rename it to Flag instead, in my humble opinion.
About the reasons to use the bitflags - the moment we need to add another variant to this enum - there will be the reason. As long as it is just one flag, not many reasons.
There was a problem hiding this comment.
On the second thought, RedisCommandFlags is also called Flags. This thing is that those are indeed flags but there is no point of using bitmap, it just for parsing user input.
There was a problem hiding this comment.
When we do not need it as flags we use bitflags, like in here:
redismodule-rs/src/context/commands.rs
Line 22 in a6156e3
There was a problem hiding this comment.
When we do not need it as flags we use bitflags, like in here:
redismodule-rs/src/context/commands.rs
Line 22 in a6156e3
The place you referred to should also have it as Flag :-) Look at the examples here. All the enum implementations usually follow the singular naming convention, not the plural. Anyway, I don't insist on not following it. It is just that in the C/C++ world, we always use the plural form, and in Rust - singular, many people get confused when switching between those.
2 participants
The new flag is an enterprise only flag. The new flag will make sure the command will not appeared on slow log,
commandcommand, and other location where we do not want to expose it.