Skip to content

Replace miniconda3 with miniforge#53436

Merged
aslonnie merged 4 commits intoray-project:masterfrom
czgdp1807:miniforge
Jun 12, 2025
Merged

Replace miniconda3 with miniforge#53436
aslonnie merged 4 commits intoray-project:masterfrom
czgdp1807:miniforge

Conversation

@czgdp1807
Copy link
Copy Markdown
Contributor

@czgdp1807 czgdp1807 commented May 30, 2025

Why are these changes needed?

I am working on this PR to replace miniconda3 with miniforge as part of the work with Quansight.

cc: @peytondmurray

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@czgdp1807 czgdp1807 force-pushed the miniforge branch 4 times, most recently from 7f5b932 to 957dd04 Compare May 30, 2025 16:39
@czgdp1807 czgdp1807 marked this pull request as ready for review May 30, 2025 18:21
@czgdp1807 czgdp1807 requested review from a team as code owners May 30, 2025 18:21
@czgdp1807 czgdp1807 requested a review from peytondmurray May 30, 2025 18:21
@aslonnie aslonnie added the go add ONLY when ready to merge, run all tests label May 30, 2025
@czgdp1807
Copy link
Copy Markdown
Contributor Author

@aslonnie This is ready to go.

@czgdp1807 czgdp1807 requested a review from aslonnie June 1, 2025 19:28
Copy link
Copy Markdown
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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

seems that it is hitting some errors on macos:
https://buildkite.com/ray-project/premerge/builds/41010#01972d74-343a-4d7a-85e8-0332a731aab8

maybe split the linux parts (the forge ones and verify-linux-wheels.sh one) into another PR? those are easier to pass and merge in.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this URL seem incorrect?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed it is. Let me fix. The tests should pass then.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lets add -sSL so that this curl does not fail silently.

Copy link
Copy Markdown
Contributor Author

@czgdp1807 czgdp1807 Jun 2, 2025

Choose a reason for hiding this comment

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

Done. Please take a look now.

@aslonnie aslonnie removed the go add ONLY when ready to merge, run all tests label Jun 2, 2025
@czgdp1807 czgdp1807 force-pushed the miniforge branch 2 times, most recently from 336f5d0 to d180637 Compare June 2, 2025 20:02
@aslonnie aslonnie added the go add ONLY when ready to merge, run all tests label Jun 2, 2025
@aslonnie aslonnie self-requested a review June 2, 2025 20:44
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this doc link also does not exist?

seems that you did a repo-wide sed replacement

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sort of. I focussed on passing the tests. The test which failed earlier probably wasn't visible to me in microcheck tests. Documentation build didn't fail so this link wasn't highlighted as well.

Thanks for the review. Let me fix the docs links everywhere.

Comment on lines 268 to 270
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I hope these are correct replacements. I have never used Miniforge 3 on Windows. Do you want me check it out on Windows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went for repository link because the other candidate - https://conda-forge.org/download/ - didn't have useful information other than links to download latest release.

@aslonnie aslonnie self-requested a review June 3, 2025 00:01
@czgdp1807
Copy link
Copy Markdown
Contributor Author

Seems like. macOS is still failing. I will update this PR to just do the Linux changes for now. Will drop the documentation changes as well since minforge migration will happen for Linux and not macOS. What do you say @aslonnie ?

@aslonnie
Copy link
Copy Markdown
Collaborator

aslonnie commented Jun 3, 2025

Seems like. macOS is still failing. I will update this PR to just do the Linux changes for now. Will drop the documentation changes as well since minforge migration will happen for Linux and not macOS. What do you say @aslonnie ?

sounds good to me. let's do it step by step. you can also just create another PR (and keep this one as a draft to iterate on)

@czgdp1807 czgdp1807 marked this pull request as draft June 3, 2025 22:02
czgdp1807 added 2 commits June 8, 2025 19:54
Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
@czgdp1807 czgdp1807 marked this pull request as ready for review June 8, 2025 15:47
@czgdp1807
Copy link
Copy Markdown
Contributor Author

@aslonnie The tests seem to pass now. May be things work. Could you please check and if possible merge?

@aslonnie
Copy link
Copy Markdown
Collaborator

fyi, the mac CI machines are currently broken. I need to fix those first, and then I can check the changes here. sorry for the delay.

@aslonnie
Copy link
Copy Markdown
Collaborator

link for myself: checking latest ci run with miniforge here: https://buildkite.com/ray-project/postmerge-macos/builds/6126

@aslonnie
Copy link
Copy Markdown
Collaborator

I am merging this for now because the aarch64 mac ones passed..

if we find something that breaks, will fix with follow ups.

@aslonnie aslonnie merged commit 50fce23 into ray-project:master Jun 12, 2025
5 of 6 checks passed
elliot-barn pushed a commit that referenced this pull request Jun 18, 2025
to be consistent with rest of the CI.

---------

Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
Co-authored-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
elliot-barn pushed a commit that referenced this pull request Jul 2, 2025
to be consistent with rest of the CI.

---------

Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
Co-authored-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants