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

[REVIEW] Pin dask and distributed for release #1003

Merged
merged 8 commits into from
Oct 3, 2022

Conversation

galipremsagar
Copy link
Contributor

@galipremsagar galipremsagar commented Sep 29, 2022

This PR pins dask and distributed to 2022.9.2 for 22.10 release.

xref: rapidsai/cudf#11822

@galipremsagar galipremsagar added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 29, 2022
@galipremsagar galipremsagar requested a review from a team as a code owner September 29, 2022 15:54
@galipremsagar galipremsagar self-assigned this Sep 29, 2022
@github-actions github-actions bot added the gpuCI gpuCI issue label Sep 29, 2022
@galipremsagar
Copy link
Contributor Author

galipremsagar commented Sep 29, 2022

@charlesbluca looks like the mix of dask-core(nightly) & dask(conda-forge) packages is biting us here again:

(cudfdev) pgali@dt07:/nvme/0/pgali/dask-cuda$ python
Python 3.9.13 | packaged by conda-forge | (main, May 27 2022, 16:56:21) 
[GCC 10.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import dask_cuda
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/nvme/0/pgali/dask-cuda/dask_cuda/__init__.py", line 12, in <module>
    from .cuda_worker import CUDAWorker
  File "/nvme/0/pgali/dask-cuda/dask_cuda/cuda_worker.py", line 12, in <module>
    from distributed import Nanny
  File "/nvme/0/pgali/envs/cudfdev/lib/python3.9/site-packages/distributed/__init__.py", line 30, in <module>
    from distributed.deploy import Adaptive, LocalCluster, SpecCluster, SSHCluster
  File "/nvme/0/pgali/envs/cudfdev/lib/python3.9/site-packages/distributed/deploy/__init__.py", line 7, in <module>
    from distributed.deploy.local import LocalCluster
  File "/nvme/0/pgali/envs/cudfdev/lib/python3.9/site-packages/distributed/deploy/local.py", line 13, in <module>
    from distributed.deploy.utils import nprocesses_nthreads
  File "/nvme/0/pgali/envs/cudfdev/lib/python3.9/site-packages/distributed/deploy/utils.py", line 6, in <module>
    from dask.utils import factors
ImportError: cannot import name 'factors' from 'dask.utils' (/nvme/0/pgali/envs/cudfdev/lib/python3.9/site-packages/dask/utils.py)

So I went ahead and added a flag-based channel(dask/label/dev) removal in our conda recipe build.

cc: @jakirkham @pentschev

@pentschev
Copy link
Member

Regarding the pinned version, it seems like there will be a release tomorrow after all: dask/community#278 . If possible, let's pin to that, we want to get dask/distributed#6996 in which will fix all the uncatched exceptions when shutting down a Dask cluster with UCX.

ci/cpu/build.sh Outdated Show resolved Hide resolved
ci/cpu/build.sh Outdated Show resolved Hide resolved
ci/gpu/build.sh Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@galipremsagar
Copy link
Contributor Author

rerun tests

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2022

Codecov Report

Base: 0.00% // Head: 0.00% // No change to project coverage 👍

Coverage data is based on head (ac1b938) compared to base (c0ae66c).
Patch has no changes to coverable lines.

Additional details and impacted files
@@             Coverage Diff              @@
##           branch-22.10   #1003   +/-   ##
============================================
  Coverage          0.00%   0.00%           
============================================
  Files                23      23           
  Lines              3102    3102           
============================================
  Misses             3102    3102           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@galipremsagar
Copy link
Contributor Author

rerun tests

ci/cpu/build.sh Outdated Show resolved Hide resolved
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.

LGTM, thanks @galipremsagar !

pip install git+https://github.com/dask/dask.git@main
pip install git+https://github.com/dask/distributed.git@main
pip install git+https://github.com/dask/dask.git@2022.9.2
pip install git+https://github.com/dask/distributed.git@2022.9.2
Copy link
Member

Choose a reason for hiding this comment

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

Maybe in a follow-up PR, but would it make more sense to use the same logic as we do in ci/gpu/build.sh, or is there a reason I'm not aware for keeping pip install?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think at this point these are redundant, I'm planning on getting these removed in 22.12 unpinning PRs across some repos which still do pip installs.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, that would be even better. Thanks Prem!

@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Oct 3, 2022
@galipremsagar
Copy link
Contributor Author

@gpucibot merge
@gpucibot merge

@galipremsagar
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit d3972fd into rapidsai:branch-22.10 Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge gpuCI gpuCI issue 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.

4 participants