-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #11168: Search believe that CFEngine agents with "dsc" in their keys are also DSC agent #1752
Conversation
be3fb12
to
9efb220
Compare
Commit modified |
9efb220
to
099b132
Compare
Commit modified |
099b132
to
4505038
Compare
Commit modified |
could you add a test with the offending key, to ensure correct behaviour ? |
|
||
val agentMap = AgentType.allValues.toList.map(a => (a.id, a :: Nil)).toMap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you didn't do what you intented here: agentMap does not contain ANY_CFENGINE, I believe you wanted to do something like:
- delete agentTypes
- val agentMap = ( (ANY_CFENGINE, "Any CFEngine based agent") :: AgentType.allValues.toList.map(a => (a.id, a :: Nil))).toMap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was on first version then eclipse havoc ... adding it back
4505038
to
96a056b
Compare
Commit modified |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding a test ? I don't remember how hard it is to do so, though.
private[this] def filterAgent(agent: String) = SUB(A_AGENTS_NAME, null, Array(agent), null) | ||
private[this] def filterAgent(agent: AgentType) = | ||
SUB(A_AGENTS_NAME, null, Array(s""""agentType":"${agent.id}""""), null) :: | ||
EQ(A_AGENTS_NAME, agent.oldShortName) :: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment here so that we remember why we need to keep old short name here?
Commit modified |
96a056b
to
c9657a4
Compare
…ir keys are also DSC agent
Commit modified |
c9657a4
to
bd64dbe
Compare
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/11168