Change workflow to mamba #271

forsyth2 · 2023-06-15T19:20:03Z

Change workflow to mamba. Resolves #252. Equivalent of zppy's E3SM-Project/zppy#429 & E3SM-Project/zppy#428

forsyth2

@xylar @tomvothecoder I made effectively the same changes as in E3SM-Project/zppy#429 & E3SM-Project/zppy#428. Does that seem fine?

Remaining instances of pip, for reference:

$ git grep -n pip ':!docs/_build_old/*'
.github/workflows/build_workflow.yml:45:          python -m pip install --upgrade pip
.github/workflows/build_workflow.yml:46:          pip install .
.github/workflows/build_workflow.yml:71:      - name: Cache pip
.github/workflows/build_workflow.yml:75:          path: ~/.cache/pip
.github/workflows/build_workflow.yml:77:          key: ${{ runner.os }}-pip-publish-docs
.github/workflows/build_workflow.yml:79:            ${{ runner.os }}-pip-
.github/workflows/release_workflow.yml:25:      - name: Cache pip
.github/workflows/release_workflow.yml:29:          path: ~/.cache/pip
.github/workflows/release_workflow.yml:31:          key: ${{ runner.os }}-pip-publish-docs
.github/workflows/release_workflow.yml:33:            ${{ runner.os }}-pip-
conda/dev.yml:8:  - pip=22.2.2
conda/meta.yaml:14:  script: "{{ PYTHON }} -m pip install . --no-deps -vv"
conda/meta.yaml:20:    - pip
docs/source/getting_started.rst:184:Instead, the developer will ``pip install .`` to build ``zstash`` with changes
docs/source/getting_started.rst:270:        pip install .
tests/follow_symlinks.sh:115:pip install .
zstash/parallel.py:132:        # be piped to this queue instead of the screen.

forsyth2 · 2023-06-15T19:43:17Z

setup.py

-    install_requires=[
-        "fair-research-login>=0.2.6,<0.3.0",
-        "globus-sdk>=3.0.0,<4.0.0",
-        "six",
-    ],


@tomvothecoder It looks like the tests don't like this part being removed. Am I supposed to add these requirements somewhere else?

The issue is that the build step of the build_workflow.yml is using a pip environment and referencing the install_requires section from setup.py for the dependencies.

You need to replace these lines

zstash/.github/workflows/build_workflow.yml

Lines 38 to 41 in c0aadc7

- name: Set up Python

uses: actions/setup-python@v4

with:

python-version: 3.9

with the step to cache and create the conda environment (zppy example):

- name: Cache Conda uses: actions/cache@v3 env: CACHE_NUMBER: 0 with: path: ~/conda_pkgs_dir key: ${{ runner.os }}-conda-${{ env.CACHE_NUMBER }}-${{ hashFiles('conda/dev.yml') }} - name: Build Conda Environment uses: conda-incubator/setup-miniconda@v2 with: activate-environment: zstash_dev miniforge-variant: Mambaforge miniforge-version: latest use-mamba: true mamba-version: "*" environment-file: conda/dev.yml channel-priority: strict auto-update-conda: true # IMPORTANT: This needs to be set for caching to work properly! use-only-tar-bz2: true

@tomvothecoder Thanks! Now I'm getting sphinx-multiversion 0.2.4 does not exist (perhaps a typo or a missing channel). on the tests though...

And everything seems to match up with what was done for zppy, which did work.

@tomvothecoder Ah, actually zppy did not work. I missed a failing GitHub action. Fixing a syntax error in E3SM-Project/zppy#434, I end up with the same error as zstash described here.

Yes, my latest commit reverting that change makes the GitHub Actions check pass. I may keep it like that to get this merged and address this issue later in the release testing process.

The checks appeared to pass for E3SM-Project/zppy#434, but after merging, the publish-docs Action failed (https://github.com/E3SM-Project/zppy/actions/runs/5284999456/jobs/9563041218: line 1: mamba: command not found). Because of this, I think it's best to hold off on merging this PR until the sphinx-multiversion error is resolved.

xylar

Yes, at a glance it looks right to me.

xylar · 2023-06-16T10:17:51Z

@forsyth2, I don't have permission to push to this branch. Could you please cherry-pick the last 2 commits from the following?
https://github.com/xylar/zstash/commits/issue-252

mahf708 · 2023-06-16T14:11:28Z

My two (potentially out-of-the-loop) comments:

We still want to keep everything functioning if one were to use pip (think users installing a dev version in their custom envs) and this shouldn't be hard at all to obtain while still moving the main actions to mamba. The change in setup.py may break this, but I will need to look more into it.
I think keeping the docs actions running on pip is okay; I don't see a specific reason here to switch to mamba (but correct me if I am wrong). Using pip will be much lighter on the CI anyway. However, I think Xylar's commits in https://github.com/xylar/zstash/commits/issue-252 will fix the issues and this should run on mamba if we want it to.

mahf708 · 2023-06-16T14:12:41Z

setup.py

@@ -8,10 +8,5 @@
    description="Long term HPSS archiving software for E3SM",
    packages=find_packages(include=["zstash", "zstash.*"]),
    python_requires=">=3.6",
-    install_requires=[


Is zstash supposed to have zero dependencies? Should we remove these deps from conda-forge as well?

We just don't want dependencies to come from pip. I don't remember the exact details. Hopefully @forsyth2 can point us to the relevant discussion, but these were causing trouble in the recent past.

I think it was just this comment from @tomvothecoder:
E3SM-Project/zppy#397 (comment)

point us to the relevant discussion

I'm not sure if this is what you meant, but these installation requirements were introduced in #154 to enable Globus support. A couple other files subsequently had to be modified, which was done in #186.

xylar · 2023-06-16T15:06:03Z

We still want to keep everything functioning if one were to use pip (think users installing a dev version in their custom envs) and this shouldn't be hard at all to obtain while still moving the main actions to mamba. The change in setup.py may break this, but I will need to look more into it.

I think our packages basically don't and never have worked with pip. They require dependencies only available on conda-forge. I can't say whether that's true for zstash but it's true for everything else (zppy, MPAS-Analysis, mache, E3SM_Diags, etc.)

I think keeping the docs actions running on pip is okay; I don't see a specific reason here to switch to mamba (but correct me if I am wrong). Using pip will be much lighter on the CI anyway. However, I think Xylar's commits in https://github.com/xylar/zstash/commits/issue-252 will fix the issues and this should run on mamba if we want it to.

I'll leave this up to @tomvothecoder and @forsyth2. To me, it seems like an undesirable practice to use pip for CI and mamba for local testing. I also like to use the automatic API generation features in sphinx and recommend that others use that. That requires having the package installed before making the docs, so pip is out.

forsyth2 · 2023-06-16T18:14:48Z

Could you please cherry-pick the last 2 commits from the following?

Cherry-picked to this branch.

To me, it seems like an undesirable practice to use pip for CI and mamba for local testing.

I agree that it probably makes more sense to stick to one tool here.

forsyth2

Thanks for the extra commits @xylar! I think this looks good now; should I go ahead and merge?

xylar · 2023-06-16T21:36:32Z

Maybe see how it goes with appy first and merge this only after that's successful...

forsyth2 · 2023-06-16T23:57:23Z

Ok, we'll wait to see how E3SM-Project/zppy#438 does

xylar · 2023-06-17T12:21:04Z

@forsyth2, as I just commented here:
E3SM-Project/zppy#438 (comment)
I think we can merge this. The approach used here worked fine in zppy.

Change workflow to mamba

c0aadc7

forsyth2 added Documentation Files in `docs` modified DevOps CI/CD, configuration, etc. labels Jun 15, 2023

forsyth2 commented Jun 15, 2023

View reviewed changes

forsyth2 requested review from xylar and tomvothecoder June 15, 2023 19:30

forsyth2 commented Jun 15, 2023

View reviewed changes

xylar approved these changes Jun 15, 2023

View reviewed changes

forsyth2 mentioned this pull request Jun 15, 2023

Change workflow to mamba E3SM-Project/e3sm_diags#693

Merged

Address comments

8450cb8

forsyth2 force-pushed the issue-252 branch from c0baa77 to 8450cb8 Compare June 15, 2023 22:28

forsyth2 mentioned this pull request Jun 15, 2023

Fix GitHub Actions E3SM-Project/zppy#434

Merged

Revert sphinx-multiversion handling

da0807c

mahf708 reviewed Jun 16, 2023

View reviewed changes

xylar added 2 commits June 16, 2023 11:03

Get sphinx-multiversion from conda-forge, not pypi

4e6c430

Use conda environment for docs in CI

ef92f3f

forsyth2 commented Jun 16, 2023

View reviewed changes

forsyth2 merged commit dfca67d into main Jun 19, 2023

forsyth2 deleted the issue-252 branch June 19, 2023 10:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change workflow to mamba #271

Change workflow to mamba #271

forsyth2 commented Jun 15, 2023

forsyth2 left a comment

forsyth2 Jun 15, 2023

tomvothecoder Jun 15, 2023 •

edited

Loading

forsyth2 Jun 15, 2023

forsyth2 Jun 15, 2023

forsyth2 Jun 15, 2023

forsyth2 Jun 16, 2023

forsyth2 Jun 16, 2023 •

edited

Loading

xylar left a comment

xylar commented Jun 16, 2023

mahf708 commented Jun 16, 2023

mahf708 Jun 16, 2023

xylar Jun 16, 2023

xylar Jun 16, 2023

forsyth2 Jun 16, 2023 •

edited

Loading

xylar commented Jun 16, 2023

forsyth2 commented Jun 16, 2023

forsyth2 left a comment

xylar commented Jun 16, 2023

forsyth2 commented Jun 16, 2023

xylar commented Jun 17, 2023

	- name: Set up Python
	uses: actions/setup-python@v4
	with:
	python-version: 3.9

Change workflow to mamba #271

Change workflow to mamba #271

Conversation

forsyth2 commented Jun 15, 2023

forsyth2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomvothecoder Jun 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forsyth2 Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

xylar left a comment

Choose a reason for hiding this comment

xylar commented Jun 16, 2023

mahf708 commented Jun 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forsyth2 Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

xylar commented Jun 16, 2023

forsyth2 commented Jun 16, 2023

forsyth2 left a comment

Choose a reason for hiding this comment

xylar commented Jun 16, 2023

forsyth2 commented Jun 16, 2023

xylar commented Jun 17, 2023

tomvothecoder Jun 15, 2023 •

edited

Loading

forsyth2 Jun 16, 2023 •

edited

Loading

forsyth2 Jun 16, 2023 •

edited

Loading