ray-packages v2.9.1 #139

regro-cf-autotick-bot · 2024-01-19T04:06:34Z

It is very likely that the current package version for this feedstock is out of date.

Checklist before merging this PR:

Dependencies have been updated if changed: see upstream
Tests have passed
Updated license if changed and license_file is packaged

Information about this PR:

Feel free to push to the bot's branch to update this PR if needed.
The bot will almost always only open one PR per version.
The bot will stop issuing PRs if more than 3 version bump PRs generated by the bot are open. If you don't want to package a particular version please close the PR.
If you want these PRs to be merged automatically, make an issue with @conda-forge-admin,please add bot automerge in the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.
If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

Pending Dependency Version Updates

Here is a list of all the pending dependency version updates for this repo. Please double check all dependencies before merging.

Name	Upstream Version	Current Version
bazel	7.0.1
opencensus	0.11.4-1.1.13
protobuf	25.2
ray-packages	2.9.1

Dependency Analysis

We couldn't run dependency analysis due to an internal error in the bot, depfinder, or grayskull. :/ Help is very welcome!

_{This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/7579198654, please use this URL for debugging.}

conda-forge-webservices · 2024-01-19T04:06:43Z

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.

recipe/meta.yaml

mattip · 2024-01-19T06:15:11Z

setup.py changed from 2.9.0 which requires updating the patch

$ git diff -r ray-2.9.0 python/setup.py
diff --git a/python/setup.py b/python/setup.py
index 6c35efcaf6..a34a39cb50 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -254,7 +254,7 @@ if setup_spec.type == SetupType.RAY:
             "pydantic!=2.0.*,!=2.1.*,!=2.2.*,!=2.3.*,!=2.4.*,<3",
             "prometheus_client >= 0.7.1",
             "smart_open",
-            "virtualenv >=20.0.24, < 20.21.1",  # For pip runtime env.
+            "virtualenv >=20.0.24, !=20.21.1",  # For pip runtime env.
         ],
         "client": [
             # The Ray client needs a specific range of gRPC to work:

mattip · 2024-01-19T06:22:46Z

The new pinning means this should be merged only after conda-forge/conda-forge-repodata-patches-feedstock#638 is merged

timkpaine · 2024-01-19T11:01:54Z

The new pinning means this should be merged only after conda-forge/conda-forge-repodata-patches-feedstock#638 is merged

Doesn't have to really, they're independent of one another

hmaarrfk · 2024-01-19T12:29:56Z

does ray link to the grpcio library at compilation time?

mattip · 2024-01-19T12:35:57Z

Yes. ray uses bazel to build everything from source including grpcio

hmaarrfk · 2024-01-19T13:54:21Z

uses bazel to build everything from source including grpcio

if this is the case, then it should be linking statically to grpcio to avoid abi clashes like the one you are seeing.

Ideally you could reconfigure bazel to use the grpcio from conda-forge. <-- preferred.

If you use the grpcio from conda-forge you should include it in your host dependencies (which it doesn't seem like it is), and in that case, it would be pinned at runtime for the valid parameters.

https://github.com/conda-forge/tensorflow-feedstock/blob/main/recipe/meta.yaml#L207

See tensorflow which uses libgrpc which will then export the right pinnings.

https://github.com/conda-forge/grpc-cpp-feedstock/blob/main/recipe/meta.yaml#L45

Given that libgrpcio uses x.x pinnings, it seems scary to see two different versions of it in the log, but i didn't speed too much time investigating beyond this.

timkpaine · 2024-01-19T14:21:28Z

Ideally you could reconfigure bazel to use the grpcio from conda-forge. <-- preferred.

agreed, this is the best approach. unfortunately there's also some discussion on the long term future of grpc in ray, and this feedstock is not maintained by anyscale folks, so there's not a lot of appetite to make more changes (we already deal with some large patches)

mattip · 2024-01-19T15:18:39Z

We discussed using external libraries in the bazel build elsewhere. It is complicated to do in bazel version5 which ray upstream is still using. The tensorflow build has an elegant work-around that would be hard for ray to adopt. Perhaps once ray moves to bazel6 it would be easier.

hmaarrfk · 2024-01-19T15:25:48Z

if you don't use the conda-forge libgrpc these issues will keep poping up.

The folks at the libgrpc feedstock do not consider it to be abi stable beyond patch version changes.

Therefore your dependency should be one of 1.56 1.57 1.58, 1.59 or 1.60 while you make sure it matches the built in one.

timkpaine · 2024-01-19T15:28:07Z

can't really hard pin grpc runtime as it cripples ray's compatibility with the ecosystem (this is why ive been harping on getting stuff done on the upstream repo)

conda/conda#11744

apmorton · 2024-01-19T18:54:09Z

can't really hard pin grpc runtime as it cripples ray's compatibility with the ecosystem

This is effectively what unvendoring would do - the feedstock would use whatever the current global grpc pin is, and would need to be rebuilt whenever that changed.

In the meantime, we could possibly emulate the correct behavior without unvendoring by:

patching the grpc dependency in ray to be the current prevailing pin in conda-forge (1.59 in this case)
carrying grpcio as a host dependency in ray-core so the correct runtime pin gets applied

Thoughts?

hmaarrfk · 2024-01-19T18:57:58Z

This is effectively what unvendoring would do - the feedstock would use whatever the current global grpc pin is, and would need to be rebuilt whenever that changed.

correct.

Reminder, rebuilding is automatically prompted by a bot.

timkpaine · 2024-01-19T18:59:07Z

It's 1.58 which is a problem. Without updating bazel, I would expect it to just ignore whatever is on the host during build time anyway. It's best to have buy-in from anyscale as anything done here is a workaround for upstream (which should ideally allow for use of dependencies from outside bazel, discussed in #90).

apmorton · 2024-01-19T19:09:03Z

As far as I have been able to tell, there is nothing wrong with grpcio 1.58.x packages themselves.

The issue is that grpc is not ABI stable, ray's native components are being built with 1.57.1, and then the python components, through grpcio, load libgrpc $cond_forge_version. The fact that ANY version of grpc other than 1.57.x works with these ray packages is luck.

Unvendoring grpc is the correct solution, but that is non-trivial.
Pinning grpcio 1.57.x in ray-core is the correct workaround without changing anything upstream, but that negatively impacts compatibility with the rest of the conda-forge ecosystem.
Getting upstream to change to 1.59.x and pinning in ray-core would kick the can down the road.
Maintaining a patch to https://github.com/ray-project/ray/blob/master/bazel/ray_deps_setup.bzl#L234-L242 to track the conda-forge global grpc pin in this feedstock and pinning in ray-core is probably the most reliable path forward other than unvendoring grpc.

timkpaine · 2024-01-19T19:19:00Z

Yep that list sounds correct, my preference continues to be upstream over anything we do here, otherwise it will just be more things breaking periodically over time

apmorton · 2024-01-19T19:33:50Z

Yep that list sounds correct, my preference continues to be upstream over anything we do here, otherwise it will just be more things breaking periodically over time

This is package already breaks over time because of this issue, and I don't think waiting for a resolution upstream is in the best interest of the conda-forge ecosystem when we all now clearly understand the problem and have a viable workaround.

Having said that, I defer to @conda-forge/core on the correct course of action for this feedstock.

timkpaine · 2024-01-19T19:40:15Z

sure, but its also cheap and easy to do (e.g. just repodata patch for known bad versions). Unless there's a volunteer to implement and maintain bazel patching in this repo in perpetuity...

apmorton · 2024-01-19T19:49:30Z

sure, but its also cheap and easy to do (e.g. just repodata patch for known bad versions). Unless there's a volunteer to implement and maintain bazel patching in this repo in perpetuity...

I disagree. I filed #136 a week ago (after working on this on and off internally for several days) and we're still discussing the correct course of action. This is not cheap and easy.

I will take a stab at the grpc revendoring patch today and am happy to help maintain this going forward until grpcio can be unvendored upstream.

timkpaine · 2024-01-19T19:55:38Z

the solution for #136 is cheap and easy in conda-forge/conda-forge-repodata-patches-feedstock#638

we're still discussing the correct course of action

its basically all said in #90 (and for the other thread, using repodata-patches), just took a little time to re-litigate 😉

timkpaine · 2024-01-20T14:06:26Z

#145 then #140 then this PR

timkpaine · 2024-01-21T23:42:49Z

@mattip could you have one last look when you get a chance

recipe/meta.yaml

mattip · 2024-01-23T06:53:51Z

LGTM. Anyone want to take a look?

hmaarrfk · 2024-01-23T13:18:56Z

presently, this would allow grpcio 1.60 to be installed with the ray package when other packages have completed their migration.

I suggest a patch:

diff --git a/recipe/meta.yaml b/recipe/meta.yaml
index 2bf83b0..1fd4452 100644
--- a/recipe/meta.yaml
+++ b/recipe/meta.yaml
@@ -12,6 +12,9 @@ source:
     - patches/0002-Disable-making-entry-scripts.patch
     - patches/0003-Ignore-warnings-in-event.cc-and-logging.cc.patch
     - patches/0004-Remove-all-dependencies-from-setup.py.patch
+    # See https://github.com/conda-forge/ray-packages-feedstock/issues/136
+    # Keep in sync with current or active migration of libgrpc to avoid
+    {% set grpcio_version = "1.59" %}
     - patches/0006-Vendor-grpc-1.59.3.patch
     # This patch applies grpc tag v1.59.3 changes from
     # https://github.com/grpc/grpc/blob/35df344f5e17a9cb290ebf0f5b0f03ddb1ff0a97/bazel/grpc_deps.bzl#L243
@@ -161,7 +164,7 @@ outputs:
         - async-timeout
         - colorful
         - gpustat >=1.0.0
-        - grpcio
+        - grpcio {{ grpcio_version }}
         - opencensus
         - prometheus_client >=0.7.1
         - py-spy >=0.2.0
@@ -185,7 +188,7 @@ outputs:
       run:
         - python
         - {{ pin_subpackage('ray-core', exact=True) }}
-        - grpcio
+        - grpcio {{ grpcio_version }}
     test:
       imports:
         - ray

mattip · 2024-01-23T13:47:11Z

Can the {set...} line be in the middle of the patches section like that?

h-vetinari · 2024-01-23T13:50:41Z

Can the {set...} line be in the middle of the patches section like that?

You can consider {% ... %} like a C-ish preprocessor for yaml. Completely separate phase, does dumb string operations (and some very limited variable gymnastics), and all of it is gone once the yaml file is handed to the actual build phase.

recipe/meta.yaml

apmorton · 2024-01-23T14:17:04Z

recipe/meta.yaml

@@ -162,7 +164,7 @@ outputs:
        - async-timeout
        - colorful
        - gpustat >=1.0.0
-        - grpcio
+        - grpcio {{ grpcio_version }}


@hmaarrfk what is the point of these pins? All outputs depend on the identical version of ray-core, which now exports a runtime dependency on a compatible libgrpc. I don't think this extra grpcio_version thing is required.

ah i see you are right. indeed these pins are not necessary. sorry for the noise.

Please revert

mattip · 2024-01-24T12:18:12Z

Thanks all for the constructive resolution of the grcpio versioning.

updated v2.9.1

4639555

regro-cf-autotick-bot requested review from dHannasch, h-vetinari, krfricke, mattip, timkpaine and vnlitvinov as code owners January 19, 2024 04:06

Avoid incompatible grpc versions

d2913cc

timkpaine reviewed Jan 19, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

update patches

3bc85bf

don't match on any grpc patch version for 1.58

344bdab

conda/conda#11744

apmorton mentioned this pull request Jan 19, 2024

patch ray-default and ray-client to avoid 2.9.0 + grcpio1.58 conda-forge/conda-forge-repodata-patches-feedstock#638

Closed

5 tasks

apmorton mentioned this pull request Jan 19, 2024

Patch vendored grpc to 1.59.3 and export a dependency in ray-core #140

Merged

5 tasks

timkpaine mentioned this pull request Jan 20, 2024

relax pydantic pin, fixes #141 #145

Merged

5 tasks

Merge branch 'main' into 2.9.1_h06b007

ceacbb0

mattip reviewed Jan 22, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

revert pinning

b239c23

pin grcpio differently

e1b208e

apmorton reviewed Jan 23, 2024

View reviewed changes

Improve comment, remove unecessary pins

478b6f5

mattip merged commit 33169db into conda-forge:main Jan 24, 2024
14 checks passed

regro-cf-autotick-bot deleted the 2.9.1_h06b007 branch January 24, 2024 12:20

timkpaine mentioned this pull request Jun 13, 2024

ray-all and ray-core currently not installable alongside Pytorch 2.3.0 (grpc conflict) #160

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ray-packages v2.9.1 #139

ray-packages v2.9.1 #139

regro-cf-autotick-bot commented Jan 19, 2024

conda-forge-webservices bot commented Jan 19, 2024

mattip commented Jan 19, 2024

mattip commented Jan 19, 2024

timkpaine commented Jan 19, 2024 •

edited

Loading

hmaarrfk commented Jan 19, 2024

mattip commented Jan 19, 2024

hmaarrfk commented Jan 19, 2024

timkpaine commented Jan 19, 2024 •

edited

Loading

mattip commented Jan 19, 2024

hmaarrfk commented Jan 19, 2024

timkpaine commented Jan 19, 2024

apmorton commented Jan 19, 2024

hmaarrfk commented Jan 19, 2024

timkpaine commented Jan 19, 2024 •

edited

Loading

apmorton commented Jan 19, 2024 •

edited

Loading

timkpaine commented Jan 19, 2024

apmorton commented Jan 19, 2024

timkpaine commented Jan 19, 2024

apmorton commented Jan 19, 2024

timkpaine commented Jan 19, 2024 •

edited

Loading

timkpaine commented Jan 20, 2024

timkpaine commented Jan 21, 2024

mattip commented Jan 23, 2024

hmaarrfk commented Jan 23, 2024

mattip commented Jan 23, 2024

h-vetinari commented Jan 23, 2024

apmorton Jan 23, 2024

hmaarrfk Jan 23, 2024

timkpaine Jan 23, 2024

hmaarrfk Jan 23, 2024

mattip commented Jan 24, 2024

ray-packages v2.9.1 #139

ray-packages v2.9.1 #139

Conversation

regro-cf-autotick-bot commented Jan 19, 2024

Pending Dependency Version Updates

Dependency Analysis

conda-forge-webservices bot commented Jan 19, 2024

mattip commented Jan 19, 2024

mattip commented Jan 19, 2024

timkpaine commented Jan 19, 2024 • edited Loading

hmaarrfk commented Jan 19, 2024

mattip commented Jan 19, 2024

hmaarrfk commented Jan 19, 2024

timkpaine commented Jan 19, 2024 • edited Loading

mattip commented Jan 19, 2024

hmaarrfk commented Jan 19, 2024

timkpaine commented Jan 19, 2024

apmorton commented Jan 19, 2024

hmaarrfk commented Jan 19, 2024

timkpaine commented Jan 19, 2024 • edited Loading

apmorton commented Jan 19, 2024 • edited Loading

timkpaine commented Jan 19, 2024

apmorton commented Jan 19, 2024

timkpaine commented Jan 19, 2024

apmorton commented Jan 19, 2024

timkpaine commented Jan 19, 2024 • edited Loading

timkpaine commented Jan 20, 2024

timkpaine commented Jan 21, 2024

mattip commented Jan 23, 2024

hmaarrfk commented Jan 23, 2024

mattip commented Jan 23, 2024

h-vetinari commented Jan 23, 2024

apmorton Jan 23, 2024

Choose a reason for hiding this comment

hmaarrfk Jan 23, 2024

Choose a reason for hiding this comment

timkpaine Jan 23, 2024

Choose a reason for hiding this comment

hmaarrfk Jan 23, 2024

Choose a reason for hiding this comment

mattip commented Jan 24, 2024

timkpaine commented Jan 19, 2024 •

edited

Loading

timkpaine commented Jan 19, 2024 •

edited

Loading

timkpaine commented Jan 19, 2024 •

edited

Loading

apmorton commented Jan 19, 2024 •

edited

Loading

timkpaine commented Jan 19, 2024 •

edited

Loading