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

Pin dask versions in CI #260

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Pin dask versions in CI #260

merged 1 commit into from
Jun 8, 2021

Conversation

ajschmidt8
Copy link
Member

Similarly to https://github.com/rapidsai/cuml/pull/3937/files, this PR pins the version of dask and distributed that's used in CI.

@ajschmidt8 ajschmidt8 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 8, 2021
@ajschmidt8 ajschmidt8 requested a review from a team as a code owner June 8, 2021 16:20
@github-actions github-actions bot added the gpuCI label Jun 8, 2021
Copy link
Member

@pentschev pentschev left a 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.

Comment on lines +62 to +63
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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pip install "git+https://github.com/dask/[email protected]" --upgrade --no-deps
pip install "git+https://github.com/dask/[email protected]" --upgrade --no-deps

Copy link
Member

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.

Copy link
Member

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 installed is to test against main

Copy link
Member

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.

Copy link
Member

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)

Copy link
Member

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.

Comment on lines +83 to +86
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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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

Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above

Copy link
Member

@pentschev pentschev left a 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.

@ajschmidt8 ajschmidt8 merged commit 21b38dd into rapidsai:branch-21.06 Jun 8, 2021
@ajschmidt8 ajschmidt8 deleted the pin-dask branch June 8, 2021 19:58
rapids-bot bot pushed a commit that referenced this pull request Jun 10, 2021
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gpuCI improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants