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

Make paasta check call validate (Closes #128) #161

Merged
merged 4 commits into from
Jan 25, 2016
Merged

Conversation

nhandler
Copy link
Contributor

This will allow paasta check to notify users of additional errors with their service. It will catch things such as invalid yelpsoa config syntax (based on the jsonschemas) and invalid chronos schedules.

Closes #128

@nhandler nhandler self-assigned this Jan 14, 2016
@@ -308,6 +309,7 @@ def paasta_check(args):
marathon_deployments_check(service_path)
sensu_check(service, service_path)
smartstack_check(service, service_path)
paasta_validate(None, service_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm always confused when I see this kind of "None" thing in an arg. None what?
Can you kwargs this?

@nhandler
Copy link
Contributor Author

Did a bit of refactoring to make paasta_validate only take args and then call paasta_validate_soa_configs. This required adding/modifying a number of tests.

if not service_path or not os.path.isdir(service_path):
print failure("%s is not a directory" % service_path,
"http://paasta.readthedocs.org/en/latest/yelpsoa_configs.html")
return 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this follows the pattern set in the rest of the file, but I'd really like to see these 0s and 1s changed to True or False and turned to the right exit code at exit time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is fine. The reason I went for the numeric approach is to allow us to easily extend this command to have multipole failure return values that we can handle in different ways. But I am fine changing it to the cleaner True/False for now.

@solarkennedy
Copy link
Contributor

afaict this looks fine. This is one of those things that will need a manual test to be sure, then ship.

@nhandler nhandler force-pushed the paasta-check-validate branch from 04a48e1 to d18e6a8 Compare January 25, 2016 21:07
nhandler added a commit that referenced this pull request Jan 25, 2016
Make paasta check call validate (Closes #128)
@nhandler nhandler merged commit dbc98af into master Jan 25, 2016
@nhandler nhandler deleted the paasta-check-validate branch January 25, 2016 22:21
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