-
Notifications
You must be signed in to change notification settings - Fork 241
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
Mrtyler instance filter #53
Conversation
@@ -200,9 +213,20 @@ def paasta_status(args): | |||
cluster_filter = args.clusters.split(",") | |||
else: | |||
cluster_filter = None | |||
if args.instances is not None: | |||
instance_filter = args.instances.split(",") |
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.
IMO instance_filter
is a misnomer - filters are generally functions rather than lists; I'd expect 'desired_instances' or something.
Saying that, I see that we use cluster_filter
and bogus_filters
elsewhere, so I guess you're just following prior art here.
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.
Yeah I didn't invent this nomenclature.
We could call them _whitelist ?
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.
ok see this is dumb. we're still having this conversation, github. just because i pushed a commit doesn't mean we're done.
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.
_whitelist is good.
r2 - None -> [], filter -> whitelist |
"""Warns the user if the filter used is not even in the deployed | ||
|
||
def report_bogus_whitelists(whitelists, items, item_type): | ||
"""Warns the user if the whitelist used is not even in the deployed |
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.
this docstring doesn't make sense to me. """Warns the user if there are invalid values in the whitelist""" would be clearer.
v3 - More renames |
shipit |
I wanted to try out some stuff with a new instance of the reminder service, but that service has a ton of instances so doing paasta status is slow and painful. Now we can filter instances like we can filter clusters: