Skip to content

Conversation

@MaxVanDijck
Copy link
Contributor

Description

Addresses 2 requirements from issue: #59155

The Dynamic Generator was deprecated in October 2023 in favor of the newer Streaming Generator. The main rationale is that the Dynamic Generator does not support pipelining by returning values while a task is still executing.

Removes the public api usage of "dynamic" and removes documentation references to the dynamic generator

Related issues

Partial of #59155

@MaxVanDijck MaxVanDijck requested review from a team, edoakes and jjyao as code owners December 6, 2025 23:22
@MaxVanDijck MaxVanDijck force-pushed the delete-dynamic-generator branch from 5c20711 to 5487426 Compare December 6, 2025 23:23
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively removes the deprecated dynamic generator from the public API and documentation, which is a good step towards simplifying the API. The changes in the documentation and option validation are clear and correct.

I've added one comment regarding a pre-existing bug in the validation logic for num_returns that could be fixed as part of this change to make it more robust.

Additionally, it seems that the tests for the dynamic generator feature have not been removed. For example, python/ray/tests/test_generators.py contains several tests that use num_returns="dynamic". These tests will likely fail with the changes in this PR. Please make sure to remove or update any related tests to ensure the CI passes and the codebase is clean.

Comment on lines 182 to +183
lambda x: None
if (x is None or x == "dynamic" or x == "streaming" or x >= 0)
if (x is None or x == "streaming" or x >= 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The validation logic for num_returns can raise a TypeError. If a string other than 'streaming' is passed (e.g., a typo like 'streamin'), the expression x >= 0 will be evaluated, causing a TypeError: '>=' not supported between 'str' and 'int'. This will crash the option validation.

To make the validation more robust, you should explicitly check the type of x before performing the comparison.

Suggested change
lambda x: None
if (x is None or x == "dynamic" or x == "streaming" or x >= 0)
if (x is None or x == "streaming" or x >= 0)
lambda x: None
if (isinstance(x, int) and x >= 0) or x == "streaming" or x is None

(int, str, type(None)),
lambda x: None
if (x is None or x == "dynamic" or x == "streaming" or x >= 0)
if (x is None or x == "streaming" or x >= 0)
Copy link

Choose a reason for hiding this comment

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

Bug: String comparison with integer causes TypeError in validation

The validation lambda for num_returns will raise a TypeError when a string other than "streaming" is passed (like the now-deprecated "dynamic"). The condition x >= 0 is evaluated after the string checks fail, but comparing a string to an integer in Python 3 raises TypeError: '>=' not supported between instances of 'str' and 'int'. The original code had x == "dynamic" checked first, which prevented this. Now, passing an invalid string value will crash with a TypeError instead of returning the user-friendly validation error message.

Fix in Cursor Fix in Web

Signed-off-by: MaxVanDijck <[email protected]>
@ray-gardener ray-gardener bot added docs An issue or change related to documentation core Issues that should be addressed in Ray Core community-contribution Contributed by the community labels Dec 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Contributed by the community core Issues that should be addressed in Ray Core docs An issue or change related to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant