-
Notifications
You must be signed in to change notification settings - Fork 11
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
Change workflow to mamba #271
Changes from 3 commits
c0aadc7
8450cb8
da0807c
4e6c430
ef92f3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -8,10 +8,5 @@ | |||||||||
description="Long term HPSS archiving software for E3SM", | ||||||||||
packages=find_packages(include=["zstash", "zstash.*"]), | ||||||||||
python_requires=">=3.6", | ||||||||||
install_requires=[ | ||||||||||
"fair-research-login>=0.2.6,<0.3.0", | ||||||||||
"globus-sdk>=3.0.0,<4.0.0", | ||||||||||
"six", | ||||||||||
], | ||||||||||
Comment on lines
-11
to
-15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomvothecoder It looks like the tests don't like this part being removed. Am I supposed to add these requirements somewhere else? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue is that the You need to replace these lines zstash/.github/workflows/build_workflow.yml Lines 38 to 41 in c0aadc7
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomvothecoder Thanks! Now I'm getting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And everything seems to match up with what was done for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tomvothecoder Ah, actually There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||||||||||
entry_points={"console_scripts": ["zstash=zstash.main:main"]}, | ||||||||||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is zstash supposed to have zero dependencies? Should we remove these deps from conda-forge as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was just this comment from @tomvothecoder:
E3SM-Project/zppy#397 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.