pace container CI #68
Conversation
tests/main/grid/test_eta.py
Outdated
| if str(error) == "eta file not specified": | ||
| pytest.xfail("testing eta file not specified") | ||
| else: | ||
| pytest.fail(f"ERROR {error}") |
There was a problem hiding this comment.
Assuming this is just to have finer-grained test results?
|
@FlorianDeconinck, @oelbert @xyuan @fmalatino, it's been a while but the PR is ready for a final review |
README.md
Outdated
| docker run -it pace_mpich:3.8 | ||
| ``` | ||
|
|
||
| In the container, the `pace` conda environment using python version 3.8 is already activated. |
There was a problem hiding this comment.
We shall move off python 3.8. The new sanctioned python is 3.11.7. Can be logged and done afterward to no block refactor
There was a problem hiding this comment.
@FlorianDeconinck horrible of me, this is incorrect. The testing/original version of the Docker containers had the pace conda environment all set up, but the current versions do not. The users have to go through the steps themselves or execute a script in the container to set up the conda environment. This sentence has been fixed with no reference to the python version being used. That could be added if wished :)
There was a problem hiding this comment.
But the script setups python3.8 correct?
There was a problem hiding this comment.
My overall advice would be to deliver a fully installed system via the image and in that case make it a python 3.11.x. Or tech stack is fragile to the version of Python because we use the language AST capacities themselves and those are heavily tied to the version of Python used.
There was a problem hiding this comment.
Can be merged as-is. I would advise to make the container very strict on what is installed, especially on the python version. The dependency tree of our stack is large and can quickly become unwieldy and I'd advise for a strict control upon it, leaving no room to the users to alter sensible versions of packages.
|
@mlee03 would you like me to merge this as is, or do you plan on pushing again with commits regarding @FlorianDeconinck's latest comment? |
Purpose
This PR containerizes the current pace CI. Therefore, this PR resolves the issue of where to store the CI input files since all CI input files can be stored or generated in the container during Github Action.
Code changes:
Requirements changes:
N/A
Infrastructure changes:
Changes have been made to
main_unit_tests.yamlto reflect the changes to CI.Main_unit_tests.yamlhas been changed tomain_unit_tests_mpich.yamlandmain_unit_tests_openmpi.yamlwhere each workflow pulls the appropriate version of image stored in the noaa-gfdl registry and runs the test with either MPICH or OpenMPI installation of MPI. Both runs the scriptbuild_and_test.shthat is found in the container.Build_and_test.shwill build the conda environment using therequirements_dev.txtandconstraints.txtfiles, generate input files, and runpytest -x /tests/mainChecklist
pace-util, HISTORY has been updatedAdditionally, if this PR contains code authored by new contributors: