Skip to content
This repository was archived by the owner on Sep 18, 2025. It is now read-only.

Remove support for UCX < 1.11.1 - #829

Merged
jakirkham merged 8 commits into
rapidsai:branch-0.25from
pentschev:remove-legacy-ucx-support
Feb 23, 2022
Merged

jakirkham merged 8 commits into
rapidsai:branch-0.25from
pentschev:remove-legacy-ucx-support

Conversation

@pentschev

@pentschev pentschev commented Jan 18, 2022

Copy link
Copy Markdown
Member

About ~1000 fewer lines of fun!

@pentschev
pentschev requested review from a team as code owners January 18, 2022 22:11
@quasiben

Copy link
Copy Markdown
Member

Woot!

@pentschev
pentschev marked this pull request as draft January 18, 2022 22:14
@quasiben

Copy link
Copy Markdown
Member

We'll need @rapidsai/admins to merge due to the README changes

@pentschev

Copy link
Copy Markdown
Member Author

Marking as draft while working on Dask-CUDA/Distributed PRs.

@pentschev

Copy link
Copy Markdown
Member Author

We'll need @rapidsai/admins to merge due to the README changes

I think you mean @rapidsai/ops-codeowners , they only need to approve AFAIK, we can merge once they and @rapidsai/ucxpy-python-codeowners both approve then.

@pentschev

Copy link
Copy Markdown
Member Author

rerun tests

@ajschmidt8 ajschmidt8 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

- pandoc=<2.0.0
- pip
- psutil
- libhwloc

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Can this filename be updated? We don't currently support Python 3.7 any longer
  2. If libhwloc is being removed here, can we also remove it from the integration repo below? It doesn't seem that any other RAPIDS libraries depend on it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

  1. Can this filename be updated? We don't currently support Python 3.7 any longer

Filename updated in 37c14fb .

  1. If libhwloc is being removed here, can we also remove it from the integration repo below? It doesn't seem that any other RAPIDS libraries depend on it.

Yes, it absolutely can be removed. I see a few other cases there, namely cloud-ml-examples and node, I'm not familiar with either of those repositories, but I believe the reason for them listing libhwloc was also due to the UCX-Py dependency. Should those be removed as well?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, it absolutely can be removed. I see a few other cases there, namely cloud-ml-examples and node, I'm not familiar with either of those repositories, but I believe the reason for them listing libhwloc was also due to the UCX-Py dependency. Should those be removed as well?

I'm not sure if cloud-ml-examples is still maintained. I'll leave that up to you for those repos. But at the very least, we should remove it from the integration repo.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Since I don't know anything about those projects, I'll leave them as is. Filed rapidsai/integration#437 to remove libhwloc support from the integration repository.

rapids-bot Bot pushed a commit to rapidsai/dask-cuda that referenced this pull request Feb 22, 2022
And about another ~500 fewer lines of fun here too.

Preparing for UCX-Py complete drop of UCX < 1.11.1 in rapidsai/ucx-py#829.

Authors:
  - Peter Andreas Entschev (https://github.com/pentschev)

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)

URL: #830
@pentschev
pentschev marked this pull request as ready for review February 22, 2022 17:08
@pentschev

Copy link
Copy Markdown
Member Author

rerun tests

@jakirkham

Copy link
Copy Markdown
Member

Would be good to update these for 3.8 & 3.9 as well

ucx-py/setup.py

Line 112 in 933b373

python_requires=">=3.6",

Maybe we could change this latter one just to 3 so we don't need to keep updating it

ucx-py/setup.py

Lines 131 to 132 in 933b373

"Programming Language :: Python :: 3.6",
"Programming Language :: Python :: 3.7",

Comment thread .readthedocs.yml Outdated
pentschev added a commit to pentschev/integration that referenced this pull request Feb 22, 2022
Since UCX < 1.11.1 support is being dropped by UCX-Py, it doesn't
require libhwloc anymore so it may be removed.

UCX-Py PR removing libhwloc: rapidsai/ucx-py#829
@pentschev

Copy link
Copy Markdown
Member Author

Would be good to update these for 3.8 & 3.9 as well

We have people using Python 3.6/3.7 still. We don't test it anymore, but it does still work and is needed by some projects so it's best to not completely block users from installing UCX-Py on older Python for now.

@pentschev

Copy link
Copy Markdown
Member Author

Maybe we could change this latter one just to 3 so we don't need to keep updating it

Good idea as well, done in 69afa10 .

@jakirkham

Copy link
Copy Markdown
Member

Would be good to update these for 3.8 & 3.9 as well

We have people using Python 3.6/3.7 still. We don't test it anymore, but it does still work and is needed by some projects so it's best to not completely block users from installing UCX-Py on older Python for now.

Interesting, who is still using UCX-Py on these older Python versions?

FWIW Python 3.6 is EOL as of 2021-12-23. So it may be worth dropping that at least.

@jakirkham

Copy link
Copy Markdown
Member

Other than that LGTM

@pentschev

Copy link
Copy Markdown
Member Author

Interesting, who is still using UCX-Py on these older Python versions?

FWIW Python 3.6 is EOL as of 2021-12-23. So it may be worth dropping that at least.

I followed up with you on that offline. Anyway, this is change is not relevant for the problem being treated in this PR, I filed #834 to do that later on.

@jakirkham

jakirkham commented Feb 23, 2022

Copy link
Copy Markdown
Member

@gpucibot merge

Edit: Forgot that doesn't work here 😅

@jakirkham
jakirkham merged commit a16f8a2 into rapidsai:branch-0.25 Feb 23, 2022
@jakirkham

Copy link
Copy Markdown
Member

Thanks Peter! 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants