-
Notifications
You must be signed in to change notification settings - Fork 197
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
Pin dask
versions in CI
#260
Conversation
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.
Picking from conda should be enough.
pip install "git+https://github.com/dask/distributed.git@2021.05.1" --upgrade --no-deps | ||
pip install "git+https://github.com/dask/dask.git@2021.05.1" --upgrade --no-deps |
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.
pip install "git+https://github.com/dask/[email protected]" --upgrade --no-deps | |
pip install "git+https://github.com/dask/[email protected]" --upgrade --no-deps |
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 shouldn't need to install again, it's already installed from conda.
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.
The reason it is being pip install
ed is to test against 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.
Yes, but we can't test against main
in branch-21.06 anymore, only branch-21.08 should do so.
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.
Fair enough. Either should be fine
The primary thing is making sure we don't forget to add this back to branch-21.08
(with main
being used)
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.
Yeah, we should follow-up with a PR immediately after this lands, as we did for other libraries yesterday.
set -x | ||
pip install "git+https://github.com/dask/distributed.git@2021.05.1" --upgrade --no-deps | ||
pip install "git+https://github.com/dask/dask.git@2021.05.1" --upgrade --no-deps | ||
set +x |
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.
set -x | |
pip install "git+https://github.com/dask/[email protected]" --upgrade --no-deps | |
pip install "git+https://github.com/dask/[email protected]" --upgrade --no-deps | |
set +x |
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.
Same comment as above
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.
Unblocking (or not, because I don't have owner permissions in RAFT), and leaving my previous comments as suggestions. Pinning or removing pip install
should have the same effect.
This PR reverts the pins that were made in #260. Those changes were only needed for the `21.06` branch. Authors: - AJ Schmidt (https://github.com/ajschmidt8) Approvers: - Peter Andreas Entschev (https://github.com/pentschev) - Dillon Cullinan (https://github.com/dillon-cullinan) URL: #264
Similarly to https://github.com/rapidsai/cuml/pull/3937/files, this PR pins the version of
dask
anddistributed
that's used in CI.