Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upMCOP-67 make runall work with compound filters #31
Conversation
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
puppetcla
commented
Jun 4, 2014
|
CLA signed by all contributors. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ploubser
Jun 6, 2014
Contributor
This is great, thank you. I think it would be useful to warn the user that the filter is being modified.
|
This is great, thank you. I think it would be useful to warn the user that the filter is being modified. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jantman
Jun 9, 2014
Contributor
@ploubser I've added a note to the usage message per your comment here and in Jira. Do you think that's sufficient, or should it puts something when the command is run as well? @ripienaar any further thoughts on this?
|
@ploubser I've added a note to the usage message per your comment here and in Jira. Do you think that's sufficient, or should it puts something when the command is run as well? @ripienaar any further thoughts on this? |
ploubser
reviewed
Jun 10, 2014
| @client.filter["compound"].clear | ||
| @client.compound_filter("puppet().enabled=true") | ||
| unless @client.filter["compound"].empty? | ||
| # munge the filter to and it with checking for enabled nodes |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ploubser
Jun 10, 2014
Contributor
It would be handy if we logged at warning or info that we were going to manipulate the filter.
ploubser
Jun 10, 2014
Contributor
It would be handy if we logged at warning or info that we were going to manipulate the filter.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ploubser
reviewed
Jun 10, 2014
| @client.compound_filter("puppet().enabled=true") | ||
| unless @client.filter["compound"].empty? | ||
| # munge the filter to and it with checking for enabled nodes | ||
| log("Modifying user-specified filter: and'ing with 'puppet().enabled=true'") |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ploubser
Jun 10, 2014
Contributor
Thanks. This is going to break the tests because runner.logger will not have been initiated. Work around would be to set runner.logger or to stub out the log method call in that specific test.
ploubser
Jun 10, 2014
Contributor
Thanks. This is going to break the tests because runner.logger will not have been initiated. Work around would be to set runner.logger or to stub out the log method call in that specific test.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Oops. Thanks for catching that. Updating. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
ploubser
Jun 11, 2014
Contributor
Made two last comments to the commit. This looks great. When you're done, if you could rebase this into a single commit I will be happy to merge it.
Thanks!
|
Made two last comments to the commit. This looks great. When you're done, if you could rebase this into a single commit I will be happy to merge it. Thanks! |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jantman
Jun 11, 2014
Contributor
Done, squashed down to one commit. Just waiting on Travis to confirm.
Thanks!
|
Done, squashed down to one commit. Just waiting on Travis to confirm. Thanks! |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Merging, thank you! |
jantman commentedJun 4, 2014
... by munging the compound filter array to sandwich the user-specified filter inside an and'ed "puppet.enabled()==true" filter
This would be a lot easier if there was a way to get the compound_filter (-S) string from the client, but there doesn't seem to be... it's converted into a filter array by the Matcher before we have access to the client. So, the best I could come up with is to do some rather unsightly array munging on the compound filter array.
This performs the equivalent to sandwiching the user-specified filter between "puppet.enabled()==true and (" and ")"