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

Two cupid_webpage fixes #201

Merged
merged 1 commit into from
Mar 7, 2025
Merged

Two cupid_webpage fixes #201

merged 1 commit into from
Mar 7, 2025

Conversation

mnlevy1981
Copy link
Collaborator

@mnlevy1981 mnlevy1981 commented Mar 7, 2025

  1. Depending on how your environment is configured, you either need to import util or cupid.util (and git_helper or cupid.git_helper); added try / except blocks for both imports
  2. Don't update github_pages_dir to a realpath unless it is non-empty (want to keep default behavior of not doing anything if user doesn't specify -g option; I was getting
RuntimeError: When specifying -g/--github-pages-dir, you must also provide -n/--name

even when not specifying -g)

Description of changes:

  • Please add an explanation of what your changes do and why you'd like us to include them.

All PRs Checklist:

1. Depending on how your environment is configured, you either need to import
util or cupid.util (and git_helper or cupid.git_helper); added try / except
blocks for both imports
2. Don't update github_pages_dir to a realpath unless it is non-empty (want to
keep default behavior of not doing anything if user doesn't specify -g option;
I was getting

RuntimeError: When specifying -g/--github-pages-dir, you must also provide -n/--name

even when not specifying -g)
@mnlevy1981
Copy link
Collaborator Author

@dabail10 reported this issue, and I was able to recreate it locally. My testing was just to run cupid-diagnostics and cupid-webpage from key_metrics; I didn't try cupid-webpage with the github pages link, or through the CESM Workflow)

Copy link
Collaborator

@dabail10 dabail10 left a comment

Choose a reason for hiding this comment

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

I tested this in my sandbox and it worked.

Copy link
Member

@samsrabin samsrabin left a comment

Choose a reason for hiding this comment

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

Sorry about that! Approved.

Copy link
Collaborator

@TeaganKing TeaganKing left a comment

Choose a reason for hiding this comment

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

Thanks @mnlevy1981 ! I tested both the -g and regular options, and everything worked as expected (commands were cupid-webpage and cupid-webpage -g ../../../../samr_cupid/land_cupid_publish_test -n landtest13). Sounds like you're also testing in the CESM workflow, so I'll let you double check that one.

@TeaganKing TeaganKing added the bug Something isn't working label Mar 7, 2025
@mnlevy1981
Copy link
Collaborator Author

I verified that the CESM Workflow showed the same

RuntimeError: When specifying -g/--github-pages-dir, you must also provide -n/--name

reported in #200, but switching to this branch fixed it.

@mnlevy1981 mnlevy1981 merged commit feee167 into NCAR:main Mar 7, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default cupid-webpage arguments should not force users to upload to github
4 participants