7.improve CIworkflow #8

ilaflott · 2024-07-23T16:35:19Z

This tries to straighten out the workflow files for githubs CI/CD pipeline. Part of testing these changes is... opening this PR and watching the correct workflows run.

`environment.yml` had different package reqs than whats in `setup.py` or `meta.yaml`. changed to be consistent. `setup-python` github action gives us accesss to a python with conda in it, remove conda reqs for now. change name.

Ian Laflotte cannot spell.

…te-pip-install-pytest not convinced this will work. lets see.

…da-conda-env-create-pip-install-pytest.yml ian still can't type.

…l to conda-env-create-and-pytest.yml shorter name

remove what seems to be redundant workflow

remove unused workflow

separate steps of docs job by new line for readability

remove unused docker build workflow

remove unused workflow file

create a conda build workflow that doesn't upload anything for PR/MR. change the conda publish workflow to run only upon push to main.

update checkout action to v4, remove conda config line that i'm not sure works and is redundant anyways.

remove branch from meta.yaml

remove gir url from source field in catalogbuilder. messes with conda build call. thank you bennet

include explicit `catalogbuilder.scripts.gen_intake_gfdl` import test in `meta.yaml` for conda package

trying to absorb what create-gfdl-catalog does into conda-env-create-and-pytest to start. if i'm understanding the pipelines correctly, these steps should work with the conda env create approach, and we can re-define them in terms of the pytest suite. lets see if my first attempt here works..

ahhhhh the pytest, conda installations brought in with the python environment pointed to by github actions "setup-python" take precedence over the conda env python etc. not my favorite thing- but it does seem at least to work consistently. i hope to see most steps work after this commit.

`which pytest` upsets github ci/cd... kind of suprising

ilaflott · 2024-07-23T18:25:41Z

so, almost entirely focused on changing the workflow files here.

deletes the following, as they are unused/old/redundant

.github/workflows/main.yml
.github/workflows/import-test.yml
.github/workflows/docker-image2.yml
.github/workflows/docker-image.yml

.github/workflows/python-conda-env.yml renamed to conda-env-create-and-pytest.yml, expanded upon to include steps originally included in .github/workflows/create-gfdl-catalog.yml, which is still present for the moment, but i think we can get rid of.

meta.yaml used to specify the git_url field with the repo URL. This is a good idea in theory but results in the conda build . in the workflow files to only look at code on the main branch- undesirable!!! Thank you @bcc2761 for figuring that out and leaving good commit notes for fre-cli. Also now includes a new import test, catalogbuilder.scripts.gen_intake_gfdl

environment.yml hada a bunch of dependencies that aren't actual dependencies for the catalogbuilder package. environment.yml now only contains what catalogbuilder needs to be pip-installed + run in the environment.

ilaflott · 2024-07-23T18:28:07Z

Also, addresses #6 #7 and #11

put publish-conda.yml name field back to old value, to avoid polluting the workflows menu of this repo more than i already did...

that later pytest call isnt redundant, it uses the downloaded artifacts from earlier in the workflow. also, pretty sure the `python` call points to the right one, even if `pytest` doesn't without the path to the executable in the conda env.

remove now-redundant create-gfdl-catalog.yml. we can test all the same things in the pipeline testing the conda env create step. for the future: absorb the test script calls into the pytest suite.

ilaflott · 2024-07-23T18:57:04Z

I'm happy with the current tweaks.

Note that all of this is changes regarding the workflow and/or environment files. There are no changes to the code of the package itself.

.github/workflows/conda-env-create-and-pytest.yml

aradhakrishnanGFDL · 2024-07-23T23:23:01Z

.github/workflows/conda-env-create-and-pytest.yml

@@ -0,0 +1,70 @@
+name: conda-env-create-and-pytest


Suggested change

name: conda-env-create-and-pytest

name: catalog-build-pytests-in-conda-env

the workflow file could also be renamed to catalog-build-pytests-in-conda-env? (or anything that is a bit descriptive as this is an important dynamic catalog building test)

i'm OK with finding a new name, but let's avoid "build" in this context. build is very overloaded here. we have catalogbuilder building catalogs, a pipeline building an environment, a pipeline building a package using conda build...

ideally, some day, this dynamic catalog creation test exists within a pytest suite

how about conda-env-create-run-tests?

aradhakrishnanGFDL · 2024-07-23T23:33:50Z

meta.yaml

    - catalogbuilder.cats
+    - catalogbuilder.scripts


@Ciheim will be curious to see if fre-cli can still take the --config option because I had not moved configs during this restructuring. We can move it if fre-cli requires it, though it shouldn't be required. Please open a separate issue if fre-cli needs catalogbuilder.config

aradhakrishnanGFDL

Thank you @ilaflott! Minor changes and suggestions made.
For a later time- This change makes me wonder if a similar, but minimal test to use the catalogbuilder conda package and run the catalog builder might add value as well, upon push. But, I can also see how the analysis-scripts repo might help with this post PR (push) tests.

aradhakrishnanGFDL · 2024-07-24T01:37:57Z

@ilaflott not sure if its tied to this PR, but the json and csv files in here https://github.com/NOAA-GFDL/CatalogBuilder/tree/main/catalogbuilder/cats do not show up in the conda package we install/download from the noaa-gfdl channel. Do you see the same behavior?

Descending into catalogbuilder and looking at cats/ reveals that there is only the init file in there within the published conda pkg. It may have to do something with this file that may need a tweak or could be removed given the settings have changed?

Co-authored-by: Aparna Radhakrishnan <aparna.radhakrishnan@noaa.gov>

ilaflott · 2024-07-24T12:42:08Z

thanks for the excellent feedback @aradhakrishnanGFDL, let me take a peek at what you're pointing out RE json files being absent.

as for the conda env building + running test... there is merit to the idea IMO. what makes
me reconsider is the surprising amount of pain github CI/CD introduces to trying to test software within a conda env.

implementing conda env tricks for github ci/cd i figured out here: NOAA-GFDL/fre-cli#127

typo

now includes files other than `__init__.py` in `catalogbuilder/cats`

ilaflott · 2024-07-24T16:42:51Z

OK, aside from the workflow file name, i've implemented the suggested changes and added another improvement to handling calls to the conda environments resources

…getting put in a conda env's "site-packages" directory. adjust respective workflow file add pytest.ini file for assist pytest calls with finding the tests

…ast parts of it.

… change, and `<root>/intakebuilder/` --> `<root>/catalogbuilder/intakebuilder` change

ilaflott · 2024-07-24T18:19:22Z

OK great news! I managed to work the packaging structure out... initially i just wanted to move tests/ into catalogbuilder/tests/, but this caused awkward problems with sys.path manipulation for module imports in the tests... and i just couldnt stop going down that rabbit hole.

…-run-pytest.yml change file name / workflow name. change `pip install --prefix` path- shouldn't target `bin/`, but the directory containing it and e.g. `lib` and i.e. other top-level things for the env.

ilaflott added 22 commits July 23, 2024 11:58

Update environment.yml

b3ee120

`environment.yml` had different package reqs than whats in `setup.py` or `meta.yaml`. changed to be consistent. `setup-python` github action gives us accesss to a python with conda in it, remove conda reqs for now. change name.

Update environment.yml

aab9702

Ian Laflotte cannot spell.

Update and rename python-conda-env.yml to python-conda-conda-env-crea…

6cd38e8

…te-pip-install-pytest not convinced this will work. lets see.

Rename python-conda-conda-env-create-pip-install-pytest to python-con…

ec12d52

…da-conda-env-create-pip-install-pytest.yml ian still can't type.

Update and rename python-conda-conda-env-create-pip-install-pytest.ym…

bc171fd

…l to conda-env-create-and-pytest.yml shorter name

Delete .github/workflows/import-test.yml

ba59474

remove what seems to be redundant workflow

Delete .github/workflows/main.yml

ea833e1

remove unused workflow

Update documentation.yml

84fa764

separate steps of docs job by new line for readability

Delete .github/workflows/docker-image2.yml

a2c917a

remove unused docker build workflow

Delete .github/workflows/docker-image.yml

f4ab697

remove unused workflow file

Update publish-conda.yml

dd5b547

Create build-conda.yml

64fd76e

create a conda build workflow that doesn't upload anything for PR/MR. change the conda publish workflow to run only upon push to main.

Delete tests/__pycache__ directory

14b409e

Update build-conda.yml

cca61f0

Update publish-conda.yml

9d700c5

Update build-conda.yml

95e3a30

update checkout action to v4, remove conda config line that i'm not sure works and is redundant anyways.

Update meta.yaml

276517b

remove branch from meta.yaml

Update meta.yaml

8d1e85f

remove gir url from source field in catalogbuilder. messes with conda build call. thank you bennet

Update meta.yaml

11921a3

include explicit `catalogbuilder.scripts.gen_intake_gfdl` import test in `meta.yaml` for conda package

Update conda-env-create-and-pytest.yml

5b971ad

`which pytest` upsets github ci/cd... kind of suprising

ilaflott requested a review from aradhakrishnanGFDL July 23, 2024 18:10

ilaflott requested review from Ciheim and ceblanton July 23, 2024 18:26

ilaflott added 3 commits July 23, 2024 14:36

Update publish-conda.yml

8692ee6

put publish-conda.yml name field back to old value, to avoid polluting the workflows menu of this repo more than i already did...

Update conda-env-create-and-pytest.yml

42d1c44

that later pytest call isnt redundant, it uses the downloaded artifacts from earlier in the workflow. also, pretty sure the `python` call points to the right one, even if `pytest` doesn't without the path to the executable in the conda env.

Delete .github/workflows/create-gfdl-catalog.yml

6bdcf79

remove now-redundant create-gfdl-catalog.yml. we can test all the same things in the pipeline testing the conda env create step. for the future: absorb the test script calls into the pytest suite.

aradhakrishnanGFDL reviewed Jul 23, 2024

View reviewed changes

.github/workflows/conda-env-create-and-pytest.yml Outdated Show resolved Hide resolved

aradhakrishnanGFDL reviewed Jul 23, 2024

View reviewed changes

aradhakrishnanGFDL requested changes Jul 23, 2024

View reviewed changes

Update conda-env-create-and-pytest.yml

6ee1781

Co-authored-by: Aparna Radhakrishnan <aparna.radhakrishnan@noaa.gov>

ilaflott added 3 commits July 24, 2024 12:30

Update conda-env-create-and-pytest.yml

83a93ea

implementing conda env tricks for github ci/cd i figured out here: NOAA-GFDL/fre-cli#127

Update conda-env-create-and-pytest.yml

0e1ffe0

typo

Update MANIFEST.in

09ca85e

now includes files other than `__init__.py` in `catalogbuilder/cats`

ilaflott added 6 commits July 24, 2024 13:10

move tests directory into catalogbuilder- as the tests/ dir is still …

01ac1e8

…getting put in a conda env's "site-packages" directory. adjust respective workflow file add pytest.ini file for assist pytest calls with finding the tests

did i fix the packaging??? i might have fixed the packaging. or at le…

332bcb0

…ast parts of it.

change path in test_ingestion.py

bd0f8f9

SO CLOSE

fc6457c

last path needed to be fixed?

a786155

edit doc to reflect 'intakebuilder' --> 'catalogbuilder' package name…

a635910

… change, and `<root>/intakebuilder/` --> `<root>/catalogbuilder/intakebuilder` change

Update and rename conda-env-create-and-pytest.yml to conda-env-create…

d40c0be

…-run-pytest.yml change file name / workflow name. change `pip install --prefix` path- shouldn't target `bin/`, but the directory containing it and e.g. `lib` and i.e. other top-level things for the env.

aradhakrishnanGFDL self-requested a review July 25, 2024 16:15

aradhakrishnanGFDL approved these changes Jul 25, 2024

View reviewed changes

Ciheim approved these changes Jul 25, 2024

View reviewed changes

ilaflott merged commit 6bc010d into main Jul 25, 2024
3 checks passed

This was referenced Jul 25, 2024

Update publish-conda.yml #13

Closed

CI workflow improvement #7

Open

ilaflott deleted the 7.improveCIworkflow branch July 29, 2024 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7.improve CIworkflow #8

7.improve CIworkflow #8

ilaflott commented Jul 23, 2024

ilaflott commented Jul 23, 2024

ilaflott commented Jul 23, 2024

ilaflott commented Jul 23, 2024

aradhakrishnanGFDL Jul 23, 2024

aradhakrishnanGFDL Jul 23, 2024

ilaflott Jul 24, 2024

ilaflott Jul 24, 2024

aradhakrishnanGFDL Jul 23, 2024

aradhakrishnanGFDL left a comment

aradhakrishnanGFDL commented Jul 24, 2024

ilaflott commented Jul 24, 2024

ilaflott commented Jul 24, 2024

ilaflott commented Jul 24, 2024

	name: conda-env-create-and-pytest
	name: catalog-build-pytests-in-conda-env

7.improve CIworkflow #8

7.improve CIworkflow #8

Conversation

ilaflott commented Jul 23, 2024

ilaflott commented Jul 23, 2024

ilaflott commented Jul 23, 2024

ilaflott commented Jul 23, 2024

aradhakrishnanGFDL Jul 23, 2024

Choose a reason for hiding this comment

aradhakrishnanGFDL Jul 23, 2024

Choose a reason for hiding this comment

ilaflott Jul 24, 2024

Choose a reason for hiding this comment

ilaflott Jul 24, 2024

Choose a reason for hiding this comment

aradhakrishnanGFDL Jul 23, 2024

Choose a reason for hiding this comment

aradhakrishnanGFDL left a comment

Choose a reason for hiding this comment

aradhakrishnanGFDL commented Jul 24, 2024

ilaflott commented Jul 24, 2024

ilaflott commented Jul 24, 2024

ilaflott commented Jul 24, 2024