🔥 conditions et at, replacing with proper cel::Predicates & ::Expressions
#396
Conversation
Signed-off-by: Alex Snaps <[email protected]>
Signed-off-by: Alex Snaps <[email protected]>
Signed-off-by: Alex Snaps <[email protected]>
Signed-off-by: Alex Snaps <[email protected]>
Signed-off-by: Alex Snaps <[email protected]>
alexsnaps
force-pushed
the
cel_predicates
branch
from
eaa10be
to
91fbf64
Compare
Signed-off-by: Alex Snaps <[email protected]>
alexsnaps
force-pushed
the
cel_predicates
branch
from
91fbf64
to
3e7a428
Compare
Signed-off-by: Alex Snaps <[email protected]>
Signed-off-by: Alex Snaps <[email protected]>
alexsnaps
force-pushed
the
cel_predicates
branch
from
f1e9c2c
to
7742644
Compare
Signed-off-by: Alex Snaps <[email protected]>
alexsnaps
force-pushed
the
cel_predicates
branch
from
db5cea2
to
787bf96
Compare
Signed-off-by: Alex Snaps <[email protected]>
alexsnaps
force-pushed
the
cel_predicates
branch
from
787bf96
to
9edeb4e
Compare
Signed-off-by: Alex Snaps <[email protected]>
| } | ||
|
|
||
| impl Context<'_> { | ||
| pub(crate) fn new(limit: &Limit, root: String, values: &HashMap<String, String>) -> Self { |
There was a problem hiding this comment.
So, this root relates to this point from #385
Should the descriptor(s) and their entries be first class citizens and have their own root binding?
Right now, limit binds the limit info (id & name), I was thinking that possibly we could have some root binding for the descriptor entries (e.g. descriptor), which... could then also be a List and hold multiple of them and all their entries: descriptor[0].foo == '1' && descriptor[1].bar != 'bad'... See this issue here, so it'd probably be a List<HashMap<String, String>> as well then...
Whatever the case, who would set that root namespace/binding to something? I'd expect it to be the server code... but that then creates further issues in the (de)serialization paths... Also, the logic of resolving the Limit.variables() would need to account for that (along side what happens in the wasm-shim's code to lazily resolve attributes from the host).
There was a problem hiding this comment.
Also, doing so would tie releasing this with a release of the kuadrant-operator... right now, this isn't necessary.
|
Pretty big PR, but I tried to keep a somewhat meaningful history and broke it down according to the #385 |
| @@ -333,6 +334,15 @@ fn encode_counter_to_key(counter: &Counter) -> Vec<u8> { | |||
| } | |||
|
|
|||
| fn encode_limit_to_key(limit: &Limit) -> Vec<u8> { | |||
| let counter = Counter::new(limit.clone(), HashMap::default()); | |||
| // fixme this is broken! | |||
There was a problem hiding this comment.
This has always been broken... and will need revisiting! But we can't just fake a the variables values like this (and they were simply left empty before, but this won't work now, as they'd fail resolution. I could "just" use the new Counter::resolved_vars, but that would just hide the problem a little more again.
Signed-off-by: Alex Snaps <[email protected]>
| let var = "timestamp(ts).getHours()"; | ||
| let limit = Limit::new( | ||
| "", | ||
| 10, | ||
| 60, | ||
| Vec::default(), | ||
| [var.try_into().expect("failed parsing!")], | ||
| ); | ||
| let counter = Counter::new( | ||
| limit, | ||
| HashMap::from([("ts".to_string(), "2019-10-12T13:20:50.52Z".to_string())]), | ||
| ) | ||
| .expect("failed creating counter"); | ||
| assert_eq!( | ||
| counter.set_variables.get(var), | ||
| Some("13".to_string()).as_ref() | ||
| ); |
There was a problem hiding this comment.
So I guess this is now new...
Signed-off-by: Alex Snaps <[email protected]>
alexsnaps
force-pushed
the
cel_predicates
branch
from
4e2db75
to
b318247
Compare
fix #385