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

Add workflow / recipe to generate Dask/distributed pre-releases #5636

Merged
merged 12 commits into from Jan 31, 2022

Conversation

charlesbluca
Copy link
Member

As part of dask/community#76 and following up on dask/dask#8469, this PR adds:

  • Conda recipes in continuous_integration to build distributed and dask pre-release packages from the Git repo

  • A GHA workflow to build the pre-release packages as a check for PRs, and to upload these packages to the Dask channel under the dev label for pushes to main

  • Closes #xxxx

  • Tests added / passed

  • Passes pre-commit run --all-files

Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Looks good Charles! 😄

Had a few comments below.

Also have we tested this locally?

Comment on lines 46 to 47
export DASK_VERSION=`conda search --override-channels -c dask/label/dev dask-core | tail -n 1 | awk '{print $2}'`
export DASK_BUILD=`conda search --override-channels -c dask/label/dev dask-core | tail -n 1 | awk '{print $3}'`
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if we can use bash arrays to compute this once and extract each piece from that result

Copy link
Member

@jakirkham jakirkham Jan 11, 2022

Choose a reason for hiding this comment

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

Perhaps something like this would work

Suggested change
export DASK_VERSION=`conda search --override-channels -c dask/label/dev dask-core | tail -n 1 | awk '{print $2}'`
export DASK_BUILD=`conda search --override-channels -c dask/label/dev dask-core | tail -n 1 | awk '{print $3}'`
export DASK_INFO=( $(conda search --override-channels -c dask/label/dev dask-core | tail -n 1 | tr " " "\n") )
export DASK_VERSION="${DASK_INFO[1]}"
export DASK_BUILD="${DASK_INFO[2]}"

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @jakirkham 😄 was playing around with this myself, think we can get away with removing the tr command

Copy link
Member

Choose a reason for hiding this comment

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

Was thinking we should be able to avoid tr, but wasn't seeing how to do that on my Mac. Maybe Linux is better behaved 😄

continuous_integration/recipes/distributed/meta.yaml Outdated Show resolved Hide resolved
&& github.ref == 'refs/heads/main'
&& github.repository == 'dask/distributed'
env:
ANACONDA_API_TOKEN: ${{ secrets.DASK_CONDA_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

Just a note, we already set this up in issue ( dask/dask#8504 ). So it should now just work here

Co-authored-by: jakirkham <jakirkham@gmail.com>
Copy link
Member Author

@charlesbluca charlesbluca left a comment

Choose a reason for hiding this comment

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

I did test locally to ensure the packages built properly, will run another build and report the resulting packages (including the results of conda convert); that actually reminded me of a relevant question:

Comment on lines +69 to +73
cd build && conda convert linux-64/*.tar.bz2 -p osx-64 \
-p osx-arm64 \
-p linux-ppc64le \
-p linux-aarch64 \
-p win-64
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we include this in the testing that runs on PRs?

@charlesbluca
Copy link
Member Author

charlesbluca commented Jan 12, 2022

Ran the commands locally, we end up getting the following packages in build:

$ find -type f -name "*.tar.bz2"
./win-64/distributed-2021.12.0a220112-py310_g3abe94e4_28.tar.bz2
./linux-ppc64le/distributed-2021.12.0a220112-py310_g3abe94e4_28.tar.bz2
./osx-64/distributed-2021.12.0a220112-py310_g3abe94e4_28.tar.bz2
./noarch/dask-2021.12.0a220112-py310_g3abe94e4_28.tar.bz2
./osx-arm64/distributed-2021.12.0a220112-py310_g3abe94e4_28.tar.bz2
./linux-aarch64/distributed-2021.12.0a220112-py310_g3abe94e4_28.tar.bz2
./linux-64/distributed-2021.12.0a220112-py310_g3abe94e4_28.tar.bz2

Anything we're missing?

EDIT:

I notice that we're not building Distributed for 3.7-3.9 - this is unexpected to me, as I would think that specifying architecture in the build would be enough to build packages for these other versions; is there something else we need to specify here?

EDIT 2:

Completely forgot that we need a build config to control the python versions we build 😅

@charlesbluca
Copy link
Member Author

Looks like we should be building all the required packages now:

$ find -type f -name "*.tar.bz2"
./win-64/distributed-2021.12.0a220112-py310_g1d4bd481_31.tar.bz2
./win-64/distributed-2021.12.0a220112-py39_g1d4bd481_31.tar.bz2
./win-64/distributed-2021.12.0a220112-py37_g1d4bd481_31.tar.bz2
./win-64/distributed-2021.12.0a220112-py38_g1d4bd481_31.tar.bz2
./linux-ppc64le/distributed-2021.12.0a220112-py310_g1d4bd481_31.tar.bz2
./linux-ppc64le/distributed-2021.12.0a220112-py39_g1d4bd481_31.tar.bz2
./linux-ppc64le/distributed-2021.12.0a220112-py37_g1d4bd481_31.tar.bz2
./linux-ppc64le/distributed-2021.12.0a220112-py38_g1d4bd481_31.tar.bz2
./osx-64/distributed-2021.12.0a220112-py310_g1d4bd481_31.tar.bz2
./osx-64/distributed-2021.12.0a220112-py39_g1d4bd481_31.tar.bz2
./osx-64/distributed-2021.12.0a220112-py37_g1d4bd481_31.tar.bz2
./osx-64/distributed-2021.12.0a220112-py38_g1d4bd481_31.tar.bz2
./noarch/dask-2021.12.0a220112-py310_g1d4bd481_31.tar.bz2
./osx-arm64/distributed-2021.12.0a220112-py310_g1d4bd481_31.tar.bz2
./osx-arm64/distributed-2021.12.0a220112-py39_g1d4bd481_31.tar.bz2
./osx-arm64/distributed-2021.12.0a220112-py37_g1d4bd481_31.tar.bz2
./osx-arm64/distributed-2021.12.0a220112-py38_g1d4bd481_31.tar.bz2
./linux-aarch64/distributed-2021.12.0a220112-py310_g1d4bd481_31.tar.bz2
./linux-aarch64/distributed-2021.12.0a220112-py39_g1d4bd481_31.tar.bz2
./linux-aarch64/distributed-2021.12.0a220112-py37_g1d4bd481_31.tar.bz2
./linux-aarch64/distributed-2021.12.0a220112-py38_g1d4bd481_31.tar.bz2
./linux-64/distributed-2021.12.0a220112-py310_g1d4bd481_31.tar.bz2
./linux-64/distributed-2021.12.0a220112-py39_g1d4bd481_31.tar.bz2
./linux-64/distributed-2021.12.0a220112-py37_g1d4bd481_31.tar.bz2
./linux-64/distributed-2021.12.0a220112-py38_g1d4bd481_31.tar.bz2

@charlesbluca charlesbluca marked this pull request as ready for review January 12, 2022 16:36
@jakirkham
Copy link
Member

@jrbourbeau would you be able to take a look at this? JFYI not needed before the release

@jakirkham
Copy link
Member

Friendly nudge @jrbourbeau 🙂

@jakirkham
Copy link
Member

@dask/maintenance could someone please review? 🙂

Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@jsignell jsignell merged commit 788005f into dask:main Jan 31, 2022
@jakirkham
Copy link
Member

Thanks Julia! 😄

@charlesbluca
Copy link
Member Author

charlesbluca commented Jan 31, 2022

Looks like the upload itself is failing:

https://github.com/dask/distributed/runs/5012056353?check_suite_focus=true

Is secrets.DASK_CONDA_TOKEN available to this repo?

EDIT:

Just checked and it looks like it isn't - maybe we should opt to make DASK_CONDA_TOKEN an organization level secret instead of repo-specific (think only the Dask repo has access to it)?

@jakirkham
Copy link
Member

jakirkham commented Jan 31, 2022

We set it at the org level. It is here

Edit: Oh though I do note that despite being set at the org level it is only visible to one repo (dask). Have just updated it to include distributed. Let's try again. It is running here

@jakirkham
Copy link
Member

Looks like we have another issue uploading

Using "dask" as upload username
Error:  File "**.tar.bz2" does not exist
Error:  File "**.tar.bz2" does not exist
Error: Process completed with exit code 1.

@charlesbluca
Copy link
Member Author

Yeah it looks like anaconda upload doesn't allow for double asterisk wildcards - a simple (though somewhat annoying to maintain) solution to fix this now is to just upload each package directory in its command, such as:

anaconda upload linux-64/*.tar.bz2
anaconda upload windows-64/*.tar.bz2
...

I'll do this now and play around with uploading locally to see if there's a better solution

@jakirkham
Copy link
Member

JFYI this was done in PR ( #5741 ) and appears to be working. Thanks Charles for the hard work here! 😄

gjoseph92 pushed a commit to gjoseph92/distributed that referenced this pull request Feb 1, 2022
…#5636)

As part of dask/community#76 and following up on dask/dask#8469, this PR adds:

- Conda recipes in `continuous_integration` to build `distributed` and `dask` pre-release packages from the Git repo
- A GHA workflow to build the pre-release packages as a check for PRs, and to upload these packages to the Dask channel under the `dev` label for pushes to `main`
@charlesbluca charlesbluca deleted the distributed-nightly branch July 20, 2022 03:00
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.

None yet

3 participants