Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
You probably want this, otherwise if Dask gets released tomorrow everything will break again.
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.
There is technically nothing broken in
dask:mainright now (and this PR probably won't be merged until after tomorrow). This PR is meant to be paired with rapidsai/dask-upstream-testing#6, and it's essentially a proposal to preventdask:maincommits from breaking all of rapids CI. We really just want dask-upstream-testing to catch the breaking change so we can fix things without blocking all down-stream projects.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.
I will again oppose this. It has been the case in the past that with minimal testing, and sometimes even with extended testing across all RAPIDS projects (as setup today) is hard to catch errors. Now what this is going to do is to essentially NOT test all of RAPIDS against changes with
main, which could help us catching early problems and fix them (hopefully) quickly. With this change we push all of that to the time when a new release occurs, which has historically been very problematic for us, especially when those changes occur close to a RAPIDS release. Furthermore, this will not prevent issues from showing up broadly in RAPIDS' CI, it will merely shift when that happens, instead of showing various smaller breakages one at a time it will ultimately show various breakages all at once after a new release.This is another step going the exact opposite direction from what I've been working for the past 5+ years to make testing "somewhat" useful. If you want to go that direction then I'll really stop watching about any Dask testing and issues whatsoever from this moment onwards and someone will have to take full responsibility for those issues. Until recently I've been the only person in the entirety of RAPIDS who have been always on top of the state of Dask testing, and if things are setup in this way is because the experience I gathered showed this was the most balanced way given the way Dask moves, if now you want to ignore that and change to something you think is best, please take full responsibility going further.
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.
I left some relevant thoughts in rapidsai/cudf#17999.
@pentschev - Your strong opposition to the proposed plan is well-received, and will definitely be considered carefully. You are correct that the plan will not solve the (very difficult) Dask + RAPIDS maintenance problem. It will only solve the "daily CI fire-drill problem". When we adopt a new release, we are still bound to uncover new problems.
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.
At this point I'm not interested in having opinions considered or not, I want people making the decisions to stand by them, take the responsibility and ultimately come up with solutions. So far it's been the case decisions are sequentially made that oppose my recommendations, but when SHTF then I'm questioned why problems are piling up and everyone is pissed off by that. IOW, I had to take responsibility for things I recommended against.
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.
It is unacceptable for you to take responsibility for anything you recommended against. I absolutely take responsibility for the state of RAPIDS + Dask testing and integration, and I will try to make sure you don't take any heat in the future. You are a fantastic engineer, so I tend to ask for your thoughts often. I will try to be more clear that you are not personally responsible for dask-cudf/dask-cuda maintenance.