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

Python Script to upload operational files #65

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

RohanSunkarapalli
Copy link
Contributor

There are two sub-folders in listofnwmfilenames folder where one is reglar approach which follows validation of URL'S and then starts uploading the valid url's which are generated. Another approach is directly acquiring gcloud api's and uploading the files based on the url's which is comparatively much faster as it skips validation.

sepehrkrz
sepehrkrz previously approved these changes Oct 12, 2023
@RohanSunkarapalli
Copy link
Contributor Author

I've added the test-cases for the listofnwmfilenames.py. I've almost covered all the inputs, functions what are used in the script. Tried to add every possible test case.

@RohanSunkarapalli
Copy link
Contributor Author

@jameshalgren
Can you review this request?

Copy link
Contributor

@jameshalgren jameshalgren left a comment

Choose a reason for hiding this comment

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

This is definitely moving the right direction. Thanks!

Please:

  • Correct the bug in the url test so that it properly either registers the fault when requesting an invalid urlbase dict entry or use keyword arguments to make the function return the default url.
  • Re-create this PR with ONLY the test script and attach it to Add tests for all potential outputs of naming  CIROH-UA/nwmurl#6
  • Make sure not to check in the .pyc files
  • Add specific tests for the individual folders in the issue list -- combinations are generally: CONUS, alaska, puertorico, hawaii; short_range, medium_range, long_range, analysis (with ensemble members for some of those); and forcing vs. model output (model output is anything not specifically labeled as forcing).


def test_selecturlbase():
assert selecturlbase({1: "https://example.com/"}, 1) == "https://example.com/"
assert selecturlbase({1: "https://example.com/"}, 2, "default") == "default"
Copy link
Contributor

Choose a reason for hiding this comment

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

Fails. Needs to select a valid dict key, spit out the default, or some useful warning.

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