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

Improve error output when validation fails #333

Merged
merged 2 commits into from
May 25, 2022
Merged

Improve error output when validation fails #333

merged 2 commits into from
May 25, 2022

Conversation

mittz
Copy link
Contributor

@mittz mittz commented May 23, 2022

Improve the error output for readability when validation fails.

Suggestions from @nick-stroud :

  • Remove timestamps from output. These make wrapped lines hard to read and are generally clutter.
  • Get rid of white space between urls and the reference to the urls. Currently the urls appear to be floating on their own which makes it harder to know the context.
  • Consider putting suggestion to run gcloud auth application-default login in output.
  • Revisit if it is possible to only show a single error if project id fails since all others are dependent on that.

Error output:

$ ./ghpc create -l ERROR --vars project_id=b examples/hpc-cluster-small.yaml
validator test_project_exists failed
error: project ID b does not exist or your credentials do not have permission to access it

validator failures can indicate a credentials problem.
cloud credentials associated with your Google Cloud account can be generated by the following command:
  $ gcloud auth application-default login
troubleshooting info appears at:
https://github.com/GoogleCloudPlatform/hpc-toolkit/blob/main/README.md#supplying-cloud-credentials-to-terraform

validation can be configured:
- treat failures as warnings by using the create command
  with the flag "--validation-level WARNING"
- can be disabled entirely by using the create command
  with the flag "--validation-level IGNORE"
- a custom set of validators can be configured following
  instructions at:
https://github.com/GoogleCloudPlatform/hpc-toolkit/blob/main/README.md#blueprint-warnings-and-errors
validation failed due to the issues listed above

Submission Checklist

  • Have you installed and run this change against pre-commit? (pre-commit install)
  • Are all tests passing? (make tests)
  • Have you written unit tests to cover this change?
  • Is unit test coverage still above 80%?
  • Have you updated all applicable documentation?
  • Have you followed the guidelines in our Contributing document?

ghpc.go Outdated Show resolved Hide resolved
@mittz mittz marked this pull request as ready for review May 23, 2022 12:30
@cboneti cboneti requested a review from nick-stroud May 24, 2022 00:33
@nick-stroud nick-stroud self-assigned this May 24, 2022
ghpc.go Outdated Show resolved Hide resolved
pkg/config/validate.go Outdated Show resolved Hide resolved
@mittz
Copy link
Contributor Author

mittz commented May 25, 2022

I addressed the comments with minimum changes. For log flag setting, I don't have a strong opinion on the way, but I applied the way of only set this flag right before the warning statement and then unset (if that is possible) right after. because it's so close to release and available by less changes. I would appreciate if I could get your comment @nick-stroud .

This is an additional test case. I ran the following command with my active credential to see the behavior for the region and zone validators after passing the project_id validator. (I removed my project ID from the following output.)

$ export PROJECT_ID="your_project_id" # Set your project_id
$ ./ghpc create -l ERROR --vars project_id=${PROJECT_ID} --vars region=a --vars zone=b examples/hpc-cluster-small.yaml
validator test_region_exists failed
error: region a is not available in project ID or your credentials do not have permission to access it

validator test_zone_exists failed
error: zone b is not available in project ID or your credentials do not have permission to access it

validator test_zone_in_region failed
error: region a is not available in project ID or your credentials do not have permission to access it

validator failures can indicate a credentials problem.
cloud credentials associated with your Google Cloud account can be generated by the following command:
  $ gcloud auth application-default login
troubleshooting info appears at:
https://github.com/GoogleCloudPlatform/hpc-toolkit/blob/main/README.md#supplying-cloud-credentials-to-terraform

validation can be configured:
- treat failures as warnings by using the create command
  with the flag "--validation-level WARNING"
- can be disabled entirely by using the create command
  with the flag "--validation-level IGNORE"
- a custom set of validators can be configured following
  instructions at:
https://github.com/GoogleCloudPlatform/hpc-toolkit/blob/main/README.md#blueprint-warnings-and-errors
validation failed due to the issues listed above

Thank you!

@nick-stroud nick-stroud merged commit b1ab6ee into GoogleCloudPlatform:develop May 25, 2022
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.

2 participants