fix(core): write use_polars_sort to Ray DatasetContext by geoHeil · Pull Request #355 · danielgafni/dagster-ray

geoHeil · 2026-04-20T15:34:37Z

Summary

Closes #354

Changelog

RayDataExecutionOptions.use_polars has been deprecated in favor of RayDataExecutionOptions.use_polars_sort

…afni#354) Ray 2.55 deprecated `DatasetContext.use_polars` in favor of `use_polars_sort`. Writing the old attribute on every `RayDataExecutionOptions.apply()` / `apply_remote()` emitted a DeprecationWarning on every materialization. Add `use_polars_sort` as the canonical field and keep `use_polars` as a deprecated alias that forwards its value and warns once at construction, preserving the existing API. `apply()` now writes `ctx.use_polars_sort` only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Ray <2.50 does not expose `DatasetContext.use_polars_sort`, so writing it would silently create an unread attribute and drop the user's setting. Probe for the new attribute and fall back to `use_polars` on older Ray. Since `pyproject.toml` pins `ray>=2.7.0`, we need to support both. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

danielgafni · 2026-04-20T16:01:53Z

+        warnings.warn(
+            "`use_polars` is deprecated; use `use_polars_sort` instead. "
+            "The value has been forwarded to `use_polars_sort`.",
+            DeprecationWarning,
+            stacklevel=2,
+        )


Use deprecated on the old field instead

Edit: just noticed you already did it, why duplicate the warning in this case?

danielgafni · 2026-04-20T16:04:03Z

+        # Ray >=2.50 renamed `use_polars` to `use_polars_sort`; set the new name when
+        # available to avoid Ray's own DeprecationWarning, and fall back on older Ray.
+        if hasattr(ctx, "use_polars_sort"):
+            ctx.use_polars_sort = self.use_polars_sort
+        else:
+            ctx.use_polars = self.use_polars_sort


this has to be an explicit Ray version check, that's cleaner to read

It also has to check the legacy self.use_polars variant

danielgafni · 2026-04-20T16:07:31Z

also, let's add a validator to ensure that the two attributes are not set to contradicting values (True/False and False/True)

…ng values Use packaging.Version to decide between `use_polars` and `use_polars_sort` on Ray's DatasetContext, and raise when callers set the deprecated and new fields to different values instead of silently forwarding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

danielgafni · 2026-04-20T21:33:14Z

+        # `use_polars_sort`; read the effective value (the legacy field wins when
+        # both are set only because the validator has already unified them).


can you explain this comment? I am seeing the opposite in the validator: use_polars can easily be unset

in general the new parameter should be preferred - even tho we did explicitly forbid conflicts

Good catch — the comment was wrong. Fixed in 5dc380d: dropped the confusing comment

danielgafni · 2026-04-20T21:35:28Z

+## Unreleased
+
+### :bug: Bug Fixes
+
+- avoid Ray 2.55 `DatasetContext.use_polars` DeprecationWarning on every `RayDataExecutionOptions.apply()` by writing to `use_polars_sort` ([#354](https://github.com/danielgafni/dagster-ray/issues/354))
+
+### :hammer_and_wrench: Other Improvements
+
+- add `RayDataExecutionOptions.use_polars_sort`; keep `use_polars` as a deprecated alias that forwards its value and emits a `DeprecationWarning`
+


do not edit the changelog, it's auto-generated by git-cliff from conventional commits history.

You can add a ## Changelog section to the PR description for a more verbose changeling entry.

Reverted in 5dc380d — the ## Unreleased block is removed. I'll add a ## Changelog section to the PR description instead.

danielgafni · 2026-04-20T21:36:16Z

+    )
+    use_polars: bool | None = Field(
+        default=None,
+        description="Deprecated. Use `use_polars_sort` instead. Ray 2.55 renamed the underlying `DatasetContext` attribute to `use_polars_sort`.",


No need to change the description, we have the deprecated parameter for the deprecation message

Done in 5dc380d — removed the custom description; relying on the deprecated parameter alone.

danielgafni · 2026-04-20T21:37:23Z

+    use_polars: bool | None = Field(
+        default=None,
+        description="Deprecated. Use `use_polars_sort` instead. Ray 2.55 renamed the underlying `DatasetContext` attribute to `use_polars_sort`.",
+        deprecated="`use_polars` is deprecated; use `use_polars_sort` instead.",


please add "is deprecated and will be removed in dagster-ray 0.5.0"

Updated in 5dc380d: ``use_polarsis deprecated and will be removed in dagster-ray 0.5.0; useuse_polars_sort` instead.`

- revert changelog edit (auto-generated by git-cliff from commit history) - drop duplicate `description` on deprecated `use_polars` field; pydantic `deprecated=` message is the single source of truth - expand deprecation message to state removal in dagster-ray 0.5.0 - simplify `apply()` to read `self.use_polars_sort` directly — the validator already unifies the legacy field into it Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

danielgafni

It's mostly ok now but I believe there will be a bug if use_polars is set to False: param merging logic doesn't correctly handle this case because it always also checks whether use_polars_sort is set to None (which can't happen due to it being non-optional bool).

It's really surprising how this wasn't caught by one of these May tests added...

Please fix param merging logic and it should be good to go.

danielgafni · 2026-04-21T04:24:00Z

+                f"({legacy!r} vs {new!r}). `use_polars` is deprecated; set only "
+                "`use_polars_sort`."
+            )
+        if legacy is not None and new is None:


This line is dead code because new can never be None

Refactored in a735002 to use explicit "use_polars" in data / "use_polars_sort" in data key-presence checks, which removes the ambiguity you called out.

Replace ambiguous `data.get(...) is None` checks with explicit `"key" in data` checks. In `mode="before"` the validator receives the raw input dict, so `None` was overloaded to mean both "key missing" and "value is None"; key-presence is unambiguous and easier to verify at a glance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

geoHeil · 2026-04-21T06:22:41Z

Pushed a735002 reworking the validator to use key-presence checks instead of is None checks, per #355 (review).

The retry in RayClusterClient.wait_until_ready only caught urllib3.exceptions.ProtocolError, so transient apiserver unavailability that surfaced as MaxRetryError (wrapping NewConnectionError with "Connection refused") aborted the wait instead of retrying. Seen in CI when minikube briefly drops its apiserver mid-test: Failed to wait for RayCluster ray/dg-dev-... readiness ... urllib3.exceptions.NewConnectionError: Failed to establish a new connection: [Errno 111] Connection refused Catch both ProtocolError and MaxRetryError so the existing 30 x 2s retry budget covers apiserver blips as well as mid- response network drops. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

geoHeil · 2026-04-21T07:00:39Z

@danielgafni can you re-run the timeout/failing minicube test?

geoHeil and others added 2 commits April 20, 2026 17:34

danielgafni requested changes Apr 20, 2026

View reviewed changes

geoHeil and others added 2 commits April 21, 2026 05:30

test(core): cover Ray Data use_polars apply branches

9352f37

danielgafni requested changes Apr 21, 2026

View reviewed changes

geoHeil marked this pull request as ready for review April 21, 2026 06:37

danielgafni merged commit bf8b895 into danielgafni:master Apr 21, 2026
31 of 33 checks passed

		# `use_polars_sort`; read the effective value (the legacy field wins when
		# both are set only because the validator has already unified them).

Conversation

geoHeil commented Apr 20, 2026 • edited by danielgafni Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changelog

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielgafni Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielgafni commented Apr 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoHeil Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielgafni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoHeil Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geoHeil commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geoHeil commented Apr 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

geoHeil commented Apr 20, 2026 •

edited by danielgafni

Loading

danielgafni Apr 20, 2026 •

edited

Loading

geoHeil Apr 21, 2026 •

edited

Loading

geoHeil Apr 21, 2026 •

edited

Loading

geoHeil commented Apr 21, 2026 •

edited

Loading