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

Remove ucx dependency for now #963

Merged

Conversation

jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Feb 16, 2023

See #962 (comment)

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member

Thanks for the PR. You'll have to adapt build.sh as well (it might work without adaptation but it would be confusing). The commit history resp. git blame should be relatively self-explanatory I hope. :)

@jorisvandenbossche
Copy link
Member Author

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 2 commits February 16, 2023 14:39
@jakirkham
Copy link
Member

cc @kkraus14

@jakirkham
Copy link
Member

jakirkham commented Feb 16, 2023

I'm hoping we can solve this on the ucx end instead of needing this change. There is a 1.14.0 release coming up, which would be a good time to coordinate package changes.

We already have a change for libnuma in the queue ( conda-forge/ucx-split-feedstock#112 ).

For example issue ( conda-forge/ucx-split-feedstock#66 ) proposes folding the CPU & GPU builds into one build and then making cudatoolkit a run_constrained dependency (so optional) instead of run (required), which drops the need for the selector and stops pulling in cudatoolkit (by default)

@kkraus14
Copy link
Contributor

I agree that I'd much prefer to fix the UCX package than disable it in the libarrow builds if possible, but understand if there's a short term fix we need to make here in the meantime.

@jorisvandenbossche
Copy link
Member Author

Yes, this PR is only meant for "until the ucx issues is solved". But to me it seems quite unsure what would be the exact timeline for getting those fixes in for ucx 1.14 (also just being unfamiliar with the package, I can't judge that), while I would consider the pyarrow issue as a serious regression in user experience (hugely blowing up every environment with pyarrow on linux), something we should fix on the short-term IMO.

@jorisvandenbossche
Copy link
Member Author

The timeout on aarch64, is that a known issue? Or could someone restart that build?

@h-vetinari h-vetinari added the automerge Merge the PR when CI passes label Feb 17, 2023
@h-vetinari
Copy link
Member

Let's undo this regression (even though somewhat subjective), and then readd it once ucx is ready.

@github-actions github-actions bot merged commit aff58f0 into conda-forge:main Feb 17, 2023
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@jorisvandenbossche jorisvandenbossche deleted the arrow-11-remove-ucx branch February 17, 2023 12:41
@h-vetinari
Copy link
Member

FYI: now that conda-forge/ucx-split-feedstock#111 has landed, I added a PR to re-enable ucx support in #1011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants