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

Add error handling for payu checkout with non-writable laboratory #528

Merged
merged 5 commits into from
Oct 14, 2024

Conversation

jo-basevi
Copy link
Collaborator

This PR:

  • In payu checkout - initialise laboratory directories before generating any metadata. This will fail with PermissionError if directories are non-writable, and prints some information on how to recover
  • Fix a small error with changing archive symlinks when a previous directory has been deleted - added test
  • Fix a uncaught warning when running payu checkout pytests

I was running with the idea of checking out a git branch - running checks on that branch and then if they failed, reverting the checkout back to the previous branch (and delete any new branches that were created). Though I then finally realised that it would make it difficult for the user to apply fixes on the branch/tag/commit they were starting from, e.g. fix project/shortpath/laboratory in config.yaml. It would require adding command-line args to payu clone/checkout that will require modifications to config.yaml.

There is the pre-existing --laboratory command-line flag to payu clone/checkout. It doesn't modify the config.yaml so will either need to be passed to subsequent payu commands or shortpath/laboratory/project options will need to modified manually in config.yaml.

I initialised laboratory directories (using mkdir_p) rather just checking for write access using os.access, so it wouldn't fail when a laboratory base directory does not exist but it can be created by a user. I added laboratory specific error messaging to Laboratory class method, and also call laboratory.initialize() in Experiment() as when running payu setup/sweep/run it would be good to know first thing if the laboratory directory is even writable.

Closes #524

@pep8speaks
Copy link

pep8speaks commented Oct 14, 2024

Hello @jo-basevi! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-10-14 04:07:28 UTC

@coveralls
Copy link

coveralls commented Oct 14, 2024

Coverage Status

coverage: 58.517% (+0.2%) from 58.29%
when pulling ba95e01 on ACCESS-NRI:524-Checkout-project-error-bug
into 1186c46 on payu-org:master.

@jo-basevi jo-basevi marked this pull request as ready for review October 14, 2024 02:57
aidanheerdegen
aidanheerdegen previously approved these changes Oct 14, 2024
Copy link
Collaborator

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM, just a tiny suggestion, feel free to disagree/ignore.

payu/laboratory.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

LGTM

@jo-basevi jo-basevi merged commit 2faf9e7 into payu-org:master Oct 14, 2024
8 checks passed
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.

Checkout fails if PROJECT is not writeable
4 participants