Skip to content

Update ImportError tests with --pre-import#914

Merged
rapids-bot[bot] merged 1 commit intorapidsai:branch-22.06from
pentschev:update-pre_import_not_found
May 25, 2022
Merged

Update ImportError tests with --pre-import#914
rapids-bot[bot] merged 1 commit intorapidsai:branch-22.06from
pentschev:update-pre_import_not_found

Conversation

@pentschev
Copy link
Member

@pentschev pentschev commented May 20, 2022

As of dask/distributed#6363, there is a change
in behavior on how plugin errors are raised.

@pentschev
Update ImportError tests with --pre-import
30e209b 
As of dask/distributed#6363, there is a change
in behavior on how plugin errors are raised.
@github-actions github-actions bot added the python python code needed label May 20, 2022
@pentschev pentschev mentioned this pull request May 20, 2022
Merged
1 task
@pentschev pentschev added bug Something isn't working 0 - Blocked Cannot progress due to external reasons non-breaking Non-breaking change labels May 20, 2022
@pentschev pentschev marked this pull request as ready for review May 20, 2022 21:06
@pentschev pentschev requested a review from a team as a code owner May 20, 2022 21:06
@pentschev pentschev added the 3 - Ready for Review Ready for review by team label May 20, 2022
@pentschev
Copy link
Member Author

pentschev commented May 20, 2022

rerun tests

@codecov-commenter
Copy link

codecov-commenter commented May 20, 2022 •
edited
Loading

Codecov Report

Merging #914 (f3b8337) into branch-22.06 (497180d) will not change coverage.
The diff coverage is n/a.

❗ Current head f3b8337 differs from pull request most recent head 30e209b. Consider uploading reports for the commit 30e209b to get more accurate results

@@              Coverage Diff              @@
##           branch-22.06    #914    +/-   ##
=============================================
  Coverage          0.00%   0.00%            
=============================================
  Files                22      16     -6     
  Lines              3075    2094   -981     
=============================================
+ Misses             3075    2094   -981
Impacted Files Coverage Δ
dask_cuda/_version.py
dask_cuda/benchmarks/local_cupy.py
dask_cuda/benchmarks/local_cupy_map_overlap.py
dask_cuda/benchmarks/local_cudf_merge.py
dask_cuda/benchmarks/local_cudf_shuffle.py
dask_cuda/benchmarks/utils.py

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 497180d...30e209b. Read the comment docs.

@jakirkham
Copy link
Member

jakirkham commented May 20, 2022

This looks good Peter! Should we merge or wait for the Dask release to be cut first?

@pentschev
Copy link
Member Author

pentschev commented May 21, 2022

This looks good Peter! Should we merge or wait for the Dask release to be cut first?

That's a good question, maybe before merging we should discuss what Dask version(s) will 22.06 support next week.

@galipremsagar
Copy link
Contributor

galipremsagar commented May 23, 2022

This looks good Peter! Should we merge or wait for the Dask release to be cut first?

That's a good question, maybe before merging we should discuss what Dask version(s) will 22.06 support next week.

Is there an upstream dask/distributed breaking change coming up in this week's release? or do we want to narrow the rolling window we are supporting now?

@pentschev
Copy link
Member Author

pentschev commented May 23, 2022

Is there an upstream dask/distributed breaking change coming up in this week's release? or do we want to narrow the rolling window we are supporting now?

We don't want to narrow it I suppose, but Dask has breaking changes too frequently and that has been the case recently. It would be best to wait and be sure IMO or we can pin to the current 2022.05.0 exclusively.

@quasiben
Copy link
Member

quasiben commented May 23, 2022

Dask should have a release today or tomorrow. This is the remain issue: dask/distributed#6422 and the fix is being discussed here: dask/distributed#6423

@galipremsagar
Copy link
Contributor

galipremsagar commented May 23, 2022 •
edited
Loading

In that case, I'm okay to wait for dask to cut a release and then evaluate(pinning to =2022.05.1)?

@jakirkham
Copy link
Member

jakirkham commented May 23, 2022

Would also prefer we get the upcoming Dask release if possible

@pentschev
Copy link
Member Author

pentschev commented May 24, 2022

Should we tentatively pin to >=2022.05.0,<=2022.05.1 or should we stick to =2022.05.1?

cc @quasiben @jakirkham @galipremsagar @charlesbluca in case any of you have opinions.

@galipremsagar
Copy link
Contributor

galipremsagar commented May 24, 2022

Should we tentatively pin to >=2022.05.0,<=2022.05.1 or should we stick to =2022.05.1?

cc @quasiben @jakirkham @galipremsagar @charlesbluca in case any of you have opinions.

I'm okay with >=2022.05.0,<=2022.05.1 if dask-cuda has no issues with 2022.05.0, else would go with =2022.05.1

@pentschev
Copy link
Member Author

pentschev commented May 24, 2022

I just had a chat with @quasiben and we're concerned that we have been testing only with Dask/Distributed main branch and since there have been so many breaking changes lately it's hard to be certain that 2022.05.0 will indeed be stable. Therefore, I think the best course of action for us is to pin to =2022.05.1 rather soon to make sure we're in good state as early as possible.

@galipremsagar would you be able to do that nice job you've been always doing and pin =2022.05.1 in all relevant RAPIDS packages?

@pentschev
Copy link
Member Author

pentschev commented May 24, 2022

rerun tests

Just to be safe things are still fine after dask/distributed#6423

@galipremsagar
Copy link
Contributor

galipremsagar commented May 24, 2022

I just had a chat with @quasiben and we're concerned that we have been testing only with Dask/Distributed main branch and since there have been so many breaking changes lately it's hard to be certain that 2022.05.0 will indeed be stable. Therefore, I think the best course of action for us is to pin to =2022.05.1 rather soon to make sure we're in good state as early as possible.

@galipremsagar would you be able to do that nice job you've been always doing and pin =2022.05.1 in all relevant RAPIDS packages?

Cool. I'll start opening the pinnings to =2022.05.1

@pentschev
Copy link
Member Author

pentschev commented May 24, 2022

Timed out test potentially related to #901 (comment) , first time I see this timing out since, if it continues to occur we may consider rethinking the test.

@pentschev
Copy link
Member Author

pentschev commented May 24, 2022

rerun test

@pentschev
Copy link
Member Author

pentschev commented May 24, 2022

rerun tests

@quasiben
Copy link
Member

quasiben commented May 25, 2022

Everything is passing now. Are we good to merge ?

@galipremsagar
Copy link
Contributor

galipremsagar commented May 25, 2022

Everything is passing now. Are we good to merge ?

Yup

@quasiben
Copy link
Member

quasiben commented May 25, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 63529e8 into rapidsai:branch-22.06 May 25, 2022
@pentschev pentschev deleted the update-pre_import_not_found branch June 23, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@galipremsagar galipremsagar galipremsagar approved these changes

Assignees

No one assigned

Labels

0 - Blocked Cannot progress due to external reasons 3 - Ready for Review Ready for review by team bug Something isn't working non-breaking Non-breaking change python python code needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pentschev @codecov-commenter @jakirkham @galipremsagar @quasiben