remove unused conditions #137
Conversation
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
| 58, 109, 101, 116, 104, 111, 100, 18, 3, 71, 69, 84, 26, 38, 10, 5, 58, 112, 97, | ||
| 116, 104, 18, 29, 47, 100, 101, 102, 97, 117, 108, 116, 47, 114, 101, 113, 117, | ||
| 101, 115, 116, 47, 104, 101, 97, 100, 101, 114, 115, 47, 112, 97, 116, 104, 34, 10, | ||
| 84, 26, 14, 10, 7, 58, 109, 101, 116, 104, 111, 100, 18, 3, 71, 69, 84, 26, 38, 10, |
There was a problem hiding this comment.
@adam-cattermole I cannot explain the reasoning for this change needed (otherwise test will fail). Any hint?
There was a problem hiding this comment.
Hmm I saw something similar when looking at this recently, when I debug printed the check request I could see the order of the headers was sometimes changing - unsure exactly on the cause or if that's what you're seeing here without printing the actual struct
There was a problem hiding this comment.
I wouldn't bother too much, with the changes that @adam-cattermole is bringing in, we'll have more confidence in these tests... It's all very brittle right now. This is probably an area we might want to invest properly in. It'd go beyond this repo tho... i.e. in the test-fw itself too probably.
| } | ||
|
|
||
| fn applies(&self) -> bool { | ||
| let attribute_value = match crate::data::get_property(self.path()).unwrap() { |
There was a problem hiding this comment.
❤️ 🙌 🎉
With this call gone, you can now delete the re-export of get_property in src/data/mod.rs
And make the fn pub(super), which gives us better encapsulation and a nicer API wrt to accessing these attributes.
diff --git a/src/data/mod.rs b/src/data/mod.rs
index 0e1cc10..66f1b8d 100644
--- a/src/data/mod.rs
+++ b/src/data/mod.rs
@@ -9,5 +9,4 @@ pub use attribute::AttributeValue;
pub use cel::Expression;
pub use cel::Predicate;
-pub use property::get_property;
pub use property::Path as PropertyPath;
diff --git a/src/data/property.rs b/src/data/property.rs
index 812edf8..06ad798 100644
--- a/src/data/property.rs
+++ b/src/data/property.rs
@@ -77,7 +77,7 @@ pub(super) fn host_get_property(path: &Path) -> Result<Option<Vec<u8>>, Status>
proxy_wasm::hostcalls::get_property(path.tokens())
}
-pub fn get_property(path: &Path) -> Result<Option<Vec<u8>>, Status> {
+pub(super) fn get_property(path: &Path) -> Result<Option<Vec<u8>>, Status> {
match *path.tokens() {
["source", "remote_address"] => remote_address(),
["auth", ..] => host_get_property(&wasm_prop(path.tokens().as_slice())),
src/configuration.rs
Outdated
| @@ -7,14 +7,11 @@ use std::sync::Arc; | |||
| use crate::configuration::action_set::ActionSet; | |||
| use crate::configuration::action_set_index::ActionSetIndex; | |||
| use crate::data; | |||
| use crate::data::Predicate; | |||
| use crate::data::PropertyPath; | |||
There was a problem hiding this comment.
Can't this be removed too and get us the same benefits (with the additional same changes to data/mod.rs and making the struct pub(super)) as get_property? (or keep the use ... as PropertyPath, but not pub)
There was a problem hiding this comment.
not sure I follow. use crate::data::PropertyPath; cannot be removed. It is being used in this module as part of the SelectorItem.
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
Kuadrant operator no longer uses conditions based on
PatternExpressions: Kuadrant/kuadrant-operator#988This PR removes conditions based on
PatternExpressionson bothRouteRuleConditionsandActionThis PR removes
SelectorDataType