Skip to content

Support both ucx:// and ucxx:// prefixes in distributed-ucxx#504

Merged
rapids-bot[bot] merged 5 commits intorapidsai:branch-0.46from
pentschev:support-prefixes
Sep 5, 2025
Merged

Support both ucx:// and ucxx:// prefixes in distributed-ucxx#504
rapids-bot[bot] merged 5 commits intorapidsai:branch-0.46from
pentschev:support-prefixes

Conversation

@pentschev
Copy link
Copy Markdown
Member

Now that Distributed deprecated ucx:// in dask/distributed#9105, we should now ensure distributed-ucxx registers both backends. This is currently handled by rapids-dask-dependency, but it will ultimately be removed from there when everything is in place.

@pentschev pentschev self-assigned this Sep 4, 2025
@pentschev pentschev added feature request New feature or request non-breaking Introduces a non-breaking change distributed-ucxx labels Sep 4, 2025
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Sep 4, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@pentschev
Copy link
Copy Markdown
Member Author

/ok to test

@pentschev
Copy link
Copy Markdown
Member Author

/ok to test

@pentschev
Copy link
Copy Markdown
Member Author

/ok to test

@pentschev pentschev marked this pull request as ready for review September 4, 2025 21:15
@pentschev pentschev requested review from a team as code owners September 4, 2025 21:15
@pentschev pentschev requested a review from AyodeAwe September 4, 2025 21:15
Copy link
Copy Markdown
Member

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

Looks good

Comment on lines +5 to +9
UCXXBackend,
UCXXBackendLegacyPrefix,
UCXXConnector,
UCXXListener,
) # noqa: F401
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.

Consider adding a __all__ = ["UCXXBackend", ...], then you don't need noqa: F401

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.

This is not meant to be a public module, we need UCXXBackend/UCXXBackendLegacyPrefix to define them in pyproject.yaml, thus we don't want to have __all__ here. I think we can remove UCXXConnector and UCXXListener here, but because they're used in rapids-dask-dependency at the moment I won't do it now, so added a TODO in 7bb45e2, and will submit a draft PR to avoid missing this once this is merged.

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.

Opened #505 to revert those changes, but that cannot be done until we can remove direct usage of distributed-ucxx from rapids-dask-dependency.

@pentschev
Copy link
Copy Markdown
Member Author

Thanks Kyle and Mads for the reviews!

@pentschev
Copy link
Copy Markdown
Member Author

/merge

@rapids-bot rapids-bot bot merged commit 2a8a80b into rapidsai:branch-0.46 Sep 5, 2025
81 checks passed
@pentschev pentschev deleted the support-prefixes branch September 5, 2025 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

distributed-ucxx feature request New feature or request non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants