Fixes #11876: Broken reporting in Rudder 4.3 with standart directives #1824
Conversation
|
PR rebased |
fanf
force-pushed
the
bug_11876/broken_reporting_in_rudder_4_3_with_standart_directives
branch
from
0ac9b2d
to
b165a22
Compare
| @@ -235,7 +235,7 @@ object ExpectedReportsSerialisation { | |||
| ~ ("components" -> (d.components.map { c => | |||
| ( | |||
| ("componentName" -> c.componentName) | |||
| ~ ("cardinality" -> c.cardinality) | |||
| // ~ ("cardinality" -> c.cardinality // #10625: ignore cardinality) | |||
There was a problem hiding this comment.
I think you should remove the line
There was a problem hiding this comment.
I'm going to remove it along with https://www.rudder-project.org/redmine/issues/11940 so that the "real" uniqueVariable cleaning is in the matching PR
There was a problem hiding this comment.
OK then the pull request is ok to me :)
| , variables = expandedVars | ||
| , originalVariables = originalVars | ||
| ) | ||
| val expandedVars = Policy.mergeVars(policyVars.map( _.expandedVars.values).toList.flatten) |
There was a problem hiding this comment.
there is a change of behaviour here: policy were ordered by priority, and if equal, alphanumerically
it had an impact on the order of variable, and usage of uniqueVariable.
this is replaced here by an unordered set, for both expandedVars and originalVars, which may result in different order for both, and it breaks completely the uniqueVariable usage - unless its done somewhere else, and I failed to see it (and in this case, we don't need to use lists)
There was a problem hiding this comment.
my bad, after talking with @fanf , he explained me that policyVars are sorted, and the map().toList kept the order
There was a problem hiding this comment.
It is still sorted, but I will add comments to make it clearer.
| } else { | ||
| val variable = mergedVars.get(newVar.spec.name) match { | ||
| case None => | ||
| Variable.matchCopy(newVar, setMultivalued = true) //asIntance is ok here, I believe |
There was a problem hiding this comment.
this comment is not true anymore
| @@ -925,12 +884,15 @@ object BuildNodeConfiguration extends Loggable { | |||
| // one more thing: we need to take care of unique variables. | |||
There was a problem hiding this comment.
i'm not sure it handles unique variable there
| , originalVars = mergeVars(sorted.flatMap(_.originalVars.values)) | ||
| , trackerVariable= base.trackerVariable.spec.cloneSetMultivalued.toVariable(sorted.flatMap(_.trackerVariable.values)) | ||
| ) | ||
| val vars = sorted.map { d => PolicyVars( |
There was a problem hiding this comment.
the name vars has been confusing me seriously
|
PR rebased |
fanf
force-pushed
the
bug_11876/broken_reporting_in_rudder_4_3_with_standart_directives
branch
from
b165a22
to
ee7f2a7
Compare
| , expandedVars : Map[String, Variable] | ||
| , originalVars : Map[String, Variable] // variable with non-expanded ${node.prop etc} values | ||
| , trackerVariable : TrackerVariable | ||
| , policyVars : NonEmptyList[PolicyVars] |
|
|
||
| // now the first draft is the mode prioritary. We keep it as "main" draft | ||
| // now the first draft is the most prioritary. We keep it as "main" draft |
There was a problem hiding this comment.
Now the first draft has priority
|
This PR is not mergeable to upper versions. |
|
This PR is not mergeable to upper versions. |
|
OK, merging this PR |
|
OK, merging this PR |
fanf
deleted the
bug_11876/broken_reporting_in_rudder_4_3_with_standart_directives
branch
https://www.rudder-project.org/redmine/issues/11876