parser, resolver: Implement Replace operator :=#49
parser, resolver: Implement Replace operator :=#49qinqon merged 2 commits intonmstate-archive:mainfrom
Conversation
AlonaKaplan
left a comment
AlonaKaplan
left a comment
There was a problem hiding this comment.
Reviewed the first two commits.
|
/kind enhancement |
To implement the Replace operation the token REPLACE has to be understood by the parser and construct the proper ast.Node. Signed-off-by: Quique Llorente <ellorent@redhat.com>
|
/hold Let's not filter on replace if users need it they will have to pipe it first. |
|
|
||
| func filter(inputState map[string]interface{}, path ast.VariadicOperator, expectedNode ast.Node) (map[string]interface{}, error) { | ||
| filtered, err := applyFuncOnPath(inputState, path, expectedNode, mapContainsValue, true) | ||
| filtered, err := applyFuncOnMap(path, inputState, expectedNode, mapContainsValue, true, true) |
There was a problem hiding this comment.
I see applyFuncOnMap as an internal method that should be used only by applyFuncOnPath.
Anyway, how is this change related to the commit/PR?
There was a problem hiding this comment.
this is internal code, and we know that inputState is a map, does not make sense to call applyFuncOnPath, this way we remove one call, that is doing some reflection that is not needed here.
As first step to implement the Replace operation at the resolver this change implement path value resplacement but just for current state since it does not involve changing path.go file to traverse capture entry path ref and apply a function. The replace operation has to be able to use a capture ref path as input, also some tuning is needed at the apply functions since the modified value has to be returned and that's different from filter function. Signed-off-by: Quique Llorente <ellorent@redhat.com>
|
/hold cancel |
| sliceEmptyAfterApply = false | ||
| adjustedSlice = append(adjustedSlice, valueAfterApply) | ||
| } else if !shouldFilterSlice { | ||
| adjustedSlice = append(adjustedSlice, valueToCheck) |
There was a problem hiding this comment.
missing sliceEmptyAfterApply=false.
There was a problem hiding this comment.
Here valueAfterApply is nil we don't have to set the variable
| } | ||
|
|
||
| if len(filteredSlice) == 0 { | ||
| if sliceEmptyAfterApply { |
There was a problem hiding this comment.
I would keep it len(adjustedSlice) == 0.
There was a problem hiding this comment.
Is no tthe same, adjustedSlice can be > 0 and sliceEmptyAfterApply false.
2 participants
For the main scenario NMPolicy need to resolve
capture.base-iface-routes | routes.running.next-hop-interface:="br1"part of this expression is the replace operator:=this PR implement that operator at parser and resolver and also allow filter/replace to use a capture ref path as input so next PR implementing the pipe will involve only the parser.Thech debt:
nmpolicy/internal/path.go: