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

avoid downloading data #901

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

avoid downloading data #901

wants to merge 2 commits into from

Conversation

mahf708
Copy link
Contributor

@mahf708 mahf708 commented Nov 23, 2024

Description

Instead of downloading the data, run the testing inside a container that has the data. See above for how the container was defined.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

If applicable:

  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

Copy link
Contributor

@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.

Thank you @mahf708, this is a very clean alternative!

@xylar
Copy link
Contributor

xylar commented Nov 23, 2024

A questionable use of your Saturday, however :-)

Copy link
Contributor

@chengzhuzhang chengzhuzhang left a comment

Choose a reason for hiding this comment

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

@mahf708 I appreciate the timely solution! Could you provide some guidance on how to create new images when testing data need to be updated. I suppose the final step is to bump the image version in build_workflow.yml..

@mahf708
Copy link
Contributor Author

mahf708 commented Nov 26, 2024

@mahf708 I appreciate the timely solution! Could you provide some guidance on how to create new images when testing data need to be updated. I suppose the final step is to bump the image version in build_workflow.yml..

The info re image is in https://github.com/E3SM-Project/containers/tree/main/e3sm-diags. I copied the the download data functions from this repo (e3sm_diags) and so we would have to update that script and trigger a build. When there is a need to update them, we can submit a PR to https://github.com/E3SM-Project/containers. Most of the process is automated

@chengzhuzhang
Copy link
Contributor

Most of the process is automated

This is great news! It just occurred to me that the container should be named as e3sm_diags_testing_data or some sort. With the name e3sm_diags it can be confusing, we had built e3sm_diags (the code base) Docker containers a a long time ago...

@xylar
Copy link
Contributor

xylar commented Nov 29, 2024

@mahf708, I agree with @chengzhuzhang's suggestion to rename the container. I initially assumed e3sm_diags contained the code itself and not just the test data, which had me concerned.

@xylar
Copy link
Contributor

xylar commented Nov 29, 2024

I decided to do my best in: E3SM-Project/containers#22

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.

Discussion about use of https://web.lcrc.anl.gov/ in CI
3 participants