Skip to content
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

Paasta rollback now accepts none, one or a list of instances. #99

Merged
merged 3 commits into from
Dec 2, 2015

Conversation

zeldinha
Copy link
Contributor

Paasta rollback now accepts none, one or a list of instances. If any are wrong it will give a warning and carry on with the valid ones.

Added a bunch of tests to verify the behaviour on each case.

def validate_given_instances(service, args_instances):
"""Trying to find which instances are actually valid for the given service"""
service_instances = list_all_instances_for_service(service)
invalid_instances = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use an empty list rather than None here

@zeldinha
Copy link
Contributor Author

zeldinha commented Dec 1, 2015

I've done most of the suggested changes, thanks for the feedback! Please let me know if there's anything else that could be improved :)

).completer = lazy_choices_completer(list_services)

list_parser.set_defaults(command=paasta_rollback)


def validate_given_instances(service_instances, args_instances):
"""Trying to find which instances are actually valid for the given service"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try and be more specific in your docstrings. Here's an example:

Given two lists of instances, return the intersection and difference of those two lists.

:param service_instances: the list of instances belonging to a services
:param args_instances: the desired instances
:returns: a tuple of (common, difference), indicating instances common to the two lists, and those only in args_instances

@Rob-Johnson
Copy link
Contributor

fix and 🚢

@solarkennedy
Copy link
Contributor

ship and iterate

zeldinha added a commit that referenced this pull request Dec 2, 2015
Paasta rollback now accepts none, one or a list of instances.
@zeldinha zeldinha merged commit d28e97c into master Dec 2, 2015
@asottile asottile deleted the paasta_rollback_all_instances branch May 31, 2016 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants