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

Cleanup configure_polaris_envs.py and deploy/shared.py #209

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

altheaden
Copy link
Collaborator

@altheaden altheaden commented Jul 24, 2024

This PR includes some general cleanup that I did in configure_polaris_envs.py and deploy/bootstrap.py, as well as a slight change to the commands which install mache. The cleanup I did consisted of the following:

  • Privatized all applicable methods
  • Re-ordered some methods
  • Narrowed the scope of some try/catch blocks (such as catching a FileNotFound error instead of a more general OSError)
  • Removed some unnecessary try/catch blocks (in the case of makedirs() which has a built-in FileExistsError check)

The installation of mache now calls upon a new file, deploy/spec-bootstrap.txt instead of listing mache's dependencies directly in the configure script. This change is both to make the script more modular and to make updating this list easier.

Checklist

  • Testing comment in the PR documents testing used to verify the changes

@altheaden altheaden added clean-up deployment Changes relate to creating conda and Spack environments, and creating a load script labels Jul 24, 2024
@altheaden
Copy link
Collaborator Author

Testing

I tested this branch by creating a new polaris environment. Everything, as far as I can tell, went smoothly.

@altheaden altheaden requested a review from xylar July 24, 2024 22:11
@altheaden altheaden changed the title Cleanup configure and shared Cleanup configure_polaris_envs.py and deploy/shared.py Jul 24, 2024
Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

@altheaden, this looks excellent apart from one small thing to fix up. Once that's fixed, I'll test deployment both with the mache package from conda-forge and the local_mache cloned from a fork and branch.

configure_polaris_envs.py Outdated Show resolved Hide resolved
@altheaden altheaden force-pushed the cleanup-configure-and-shared branch from 7bb9fd2 to 2a758d7 Compare July 25, 2024 15:32
configure_polaris_envs.py Outdated Show resolved Hide resolved
@xylar
Copy link
Collaborator

xylar commented Jul 25, 2024

@altheaden, other than my comment on the comment (how meta!) above, everything looks good by inspection. I'll do the tests I said it'd do and approve unless I run into any trouble.

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

I was able to create a polaris conda environment and activation script both with and without the --mache_fork/--mache_branch options. I don't think any further testing is needed for this PR.

Thanks for working on this, @altheaden!

@altheaden
Copy link
Collaborator Author

@xylar sounds great! I'll update that comment and then merge.

Co-authored-by: Xylar Asay-Davis <xylarstorm@gmail.com>
@altheaden altheaden merged commit d04aadd into E3SM-Project:main Jul 25, 2024
4 checks passed
@altheaden altheaden deleted the cleanup-configure-and-shared branch July 25, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean-up deployment Changes relate to creating conda and Spack environments, and creating a load script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants