[serve][llm] Fix serve build YAML serialization for AggregationFunction enum by nrghosh · Pull Request #58509 · ray-project/ray

nrghosh · 2025-11-10T20:39:06Z

PR #56871 introduced the AggregationFunction enum for autoscaling metrics aggregation. However, the serve build command's YAML serialization path was not updated to handle Enum objects, causing RepresenterError when serializing AggregationFunction enum values in autoscaling config.

ex. yaml.representer.RepresenterError: ('cannot represent an object', <AggregationFunction.MEAN: 'mean'>)

This fix adds

a helper function that recursively converts Enum objects to their string values before YAML dump, ensuring proper serialization of all enum types in the configuration.
a test that
(a) Creates a temporary output YAML file
(b) Reads the config from that file
(c) Verifies that AggregationFunction.MEAN is correctly serialized as "mean" (string)

Fixes #58485

PR ray-project#56871 introduced the AggregationFunction enum for autoscaling metrics aggregation. However, the serve build command's YAML serialization path was not updated to handle Enum objects, causing RepresenterError when serializing AggregationFunction enum values in autoscaling config. This fix adds a helper function that recursively converts Enum objects to their string values before YAML dump, ensuring proper serialization of all enum types in the configuration. Fixes ray-project#58485 Signed-off-by: Nikhil Ghosh <[email protected]>

python/ray/serve/scripts.py

gemini-code-assist

Code Review

This pull request addresses an issue with YAML serialization of Enum objects in the serve build command by introducing a recursive helper function to convert enums to strings. The fix is correct and solves the problem.

I've provided a suggestion to use PyYAML's custom representers instead of a manual conversion function. This is a more idiomatic and robust approach that simplifies the code. Overall, good work on fixing this bug.

python/ray/serve/scripts.py

tgowda1996

Since we are using a dumper, we should probably add the parsing logic to it?
We can use the add_multi_representer and add one for Enum
that should take care of list, dict and other nested objects

Something like this at the end of the file

def enum_representer(dumper, data):
    return dumper.represent_scalar("tag:yaml.org,2002:str", data.value)

ServeDeploySchemaDumper.add_multi_representer(Enum, enum_representer)

Move _convert_enums_to_strings helper function before decorator chain to ensure CLI decorators are correctly applied to build() function. Addresses decorator misplacement issue identified in code review. Signed-off-by: Nikhil Ghosh <[email protected]>

nrghosh · 2025-11-13T01:14:20Z

/gemini review

Replace manual enum-to-string conversion with PyYAML's add_multi_representer. This is more idiomatic and cleaner, following the pattern used for other custom types (e.g., str_presenter). The representer automatically handles nested structures without manual recursion. Addresses feedback from code review. Signed-off-by: Nikhil Ghosh <[email protected]>

gemini-code-assist

Code Review

This pull request introduces a fix for a RepresenterError that occurs during YAML serialization in the serve build command when an AggregationFunction enum is present in the autoscaling configuration. The fix involves adding a new helper function, _convert_enums_to_strings, which recursively traverses the configuration dictionary and converts any Enum instances to their string values. This function is then applied to the configuration object before it's passed to yaml.dump.

The changes are well-implemented, targeted, and effectively resolve the described issue. The new helper function is clean, correctly handles nested data structures, and is appropriately placed. The overall fix is solid. I have no further comments. Good work!

python/ray/serve/scripts.py

nrghosh · 2025-11-14T00:06:32Z

/gemini review

gemini-code-assist

Code Review

This pull request addresses a YAML serialization issue with Enum objects in the serve build command by introducing a custom representer. The fix is correct and well-implemented. I have one suggestion to enhance type safety and maintain consistency with the existing codebase.

gemini-code-assist · 2025-11-14T00:08:19Z

python/ray/serve/scripts.py

            super().write_line_break()
+
+
+def enum_representer(dumper, data):


For better type safety and consistency with the str_presenter function already in this file, please add type hints to the enum_representer function signature.

Suggested change

def enum_representer(dumper, data):

def enum_representer(dumper: yaml.Dumper, data: Enum):

Register enum representer with yaml.SafeDumper instead of only ServeDeploySchemaDumper. This ensures enum serialization works for: - serve config command (uses yaml.safe_dump) - serve status command (uses yaml.safe_dump) - serve build command (uses ServeDeploySchemaDumper which extends SafeDumper) Fixes issue where config/status commands would fail with RepresenterError when encountering enum values in autoscaling configs. Signed-off-by: Nikhil Ghosh <[email protected]>

Add type hints to match the pattern used in str_presenter function for better type safety and code consistency. Signed-off-by: Nikhil Ghosh <[email protected]>

nrghosh · 2025-11-14T00:11:32Z

/gemini review

gemini-code-assist

Code Review

This pull request addresses a RepresenterError during YAML serialization of AggregationFunction enum values in the serve build command. The fix introduces a custom YAML representer for Enum objects, ensuring they are serialized as their string values. This is achieved by registering the representer with yaml.SafeDumper, which applies the fix to all YAML dumps in serve CLI commands.

The approach is sound. I've added one suggestion to make the new representer more robust by explicitly converting enum values to strings, which will prevent potential issues with enums that have non-string values.

python/ray/serve/scripts.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Nikhil G <[email protected]>

nrghosh · 2025-11-14T00:18:21Z

Since we are using a dumper, we should probably add the parsing logic to it? We can use the add_multi_representer and add one for Enum that should take care of list, dict and other nested objects

Something like this at the end of the file
def enum_representer(dumper, data):
    return dumper.represent_scalar("tag:yaml.org,2002:str", data.value)

ServeDeploySchemaDumper.add_multi_representer(Enum, enum_representer)

addressed, thanks 👍🏽

Signed-off-by: Nikhil Ghosh <[email protected]>

abrarsheikh · 2025-11-17T18:42:53Z

please add a test for this. Also what does dumper.represent_scalar("tag:yaml.org,2002:str", data.value) do ?

nrghosh · 2025-11-17T22:56:57Z

please add a test for this. Also what does dumper.represent_scalar("tag:yaml.org,2002:str", data.value) do ?

@abrarsheikh it creates a YAML string scalar node with the value "mean" for example (which would produce aggreation_function: mean. We have to use this because the custom representers must return a YAML node object not a raw value - so represent_scalar() creates that node with the right type tag so PyYAML can know how to format it.

dumper.represent_scalar() is PyYAML that creates a YAML scalar (primitive) node
"tag:yaml.org,2002:str" is YAML tag indicating a string

Added a test to ensure enum representer correctly serializes

abrarsheikh · 2025-11-17T23:11:05Z

python/ray/serve/tests/unit/test_cli.py

+
+    def test_autoscaling_config_with_enum_serialization(self):
+        """Test that AutoscalingConfig with AggregationFunction enum serializes correctly."""
+        from ray.serve.config import AutoscalingConfig


move imports to top of file

abrarsheikh · 2025-11-17T23:11:34Z

python/ray/serve/tests/unit/test_cli.py

+        yaml_str = yaml.safe_dump(config_dict, sort_keys=False)
+        parsed = yaml.safe_load(yaml_str)


i think we should be testing the cli build command, not yaml.safe_dump internal implementation

Signed-off-by: Nikhil Ghosh <[email protected]>

python/ray/serve/scripts.py

Signed-off-by: Nikhil Ghosh <[email protected]>

- Fix: Register Enum representer only on ServeDeploySchemaDumper to avoid global side effects - Also updated serve config and serve status to use the custom dumper. Signed-off-by: Nikhil Ghosh <[email protected]>

python/ray/serve/scripts.py

Signed-off-by: Nikhil Ghosh <[email protected]>

nrghosh

serve build tests are failing (pending fix: #58762) - unrelated to this PR

…on enum (ray-project#58509) PR ray-project#56871 introduced the AggregationFunction enum for autoscaling metrics aggregation. However, the serve build command's YAML serialization path was not updated to handle Enum objects, causing RepresenterError when serializing AggregationFunction enum values in autoscaling config. ex. `yaml.representer.RepresenterError: ('cannot represent an object', <AggregationFunction.MEAN: 'mean'>)` This fix adds 1. a helper function that recursively converts Enum objects to their string values before YAML dump, ensuring proper serialization of all enum types in the configuration. 2. a test that (a) Creates a temporary output YAML file (b) Reads the config from that file (c) Verifies that AggregationFunction.MEAN is correctly serialized as "mean" (string) Fixes ray-project#58485 --------- Signed-off-by: Nikhil Ghosh <[email protected]> Signed-off-by: Nikhil G <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…on enum (ray-project#58509) PR ray-project#56871 introduced the AggregationFunction enum for autoscaling metrics aggregation. However, the serve build command's YAML serialization path was not updated to handle Enum objects, causing RepresenterError when serializing AggregationFunction enum values in autoscaling config. ex. `yaml.representer.RepresenterError: ('cannot represent an object', <AggregationFunction.MEAN: 'mean'>)` This fix adds 1. a helper function that recursively converts Enum objects to their string values before YAML dump, ensuring proper serialization of all enum types in the configuration. 2. a test that (a) Creates a temporary output YAML file (b) Reads the config from that file (c) Verifies that AggregationFunction.MEAN is correctly serialized as "mean" (string) Fixes ray-project#58485 --------- Signed-off-by: Nikhil Ghosh <[email protected]> Signed-off-by: Nikhil G <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: YK <[email protected]>

…on enum (ray-project#58509) PR ray-project#56871 introduced the AggregationFunction enum for autoscaling metrics aggregation. However, the serve build command's YAML serialization path was not updated to handle Enum objects, causing RepresenterError when serializing AggregationFunction enum values in autoscaling config. ex. `yaml.representer.RepresenterError: ('cannot represent an object', <AggregationFunction.MEAN: 'mean'>)` This fix adds 1. a helper function that recursively converts Enum objects to their string values before YAML dump, ensuring proper serialization of all enum types in the configuration. 2. a test that (a) Creates a temporary output YAML file (b) Reads the config from that file (c) Verifies that AggregationFunction.MEAN is correctly serialized as "mean" (string) Fixes ray-project#58485 --------- Signed-off-by: Nikhil Ghosh <[email protected]> Signed-off-by: Nikhil G <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…on enum (ray-project#58509) PR ray-project#56871 introduced the AggregationFunction enum for autoscaling metrics aggregation. However, the serve build command's YAML serialization path was not updated to handle Enum objects, causing RepresenterError when serializing AggregationFunction enum values in autoscaling config. ex. `yaml.representer.RepresenterError: ('cannot represent an object', <AggregationFunction.MEAN: 'mean'>)` This fix adds 1. a helper function that recursively converts Enum objects to their string values before YAML dump, ensuring proper serialization of all enum types in the configuration. 2. a test that (a) Creates a temporary output YAML file (b) Reads the config from that file (c) Verifies that AggregationFunction.MEAN is correctly serialized as "mean" (string) Fixes ray-project#58485 --------- Signed-off-by: Nikhil Ghosh <[email protected]> Signed-off-by: Nikhil G <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: peterxcli <[email protected]>

nrghosh requested a review from a team as a code owner November 10, 2025 20:39

nrghosh mentioned this pull request Nov 10, 2025

[Serve] serve build failing for LLMConfig based applications #58485

Closed

cursor bot reviewed Nov 10, 2025

View reviewed changes

python/ray/serve/scripts.py Outdated Show resolved Hide resolved

gemini-code-assist bot reviewed Nov 10, 2025

View reviewed changes

python/ray/serve/scripts.py Outdated Show resolved Hide resolved

tgowda1996 reviewed Nov 10, 2025

View reviewed changes

Fix decorator misplacement bug

68e2765

Move _convert_enums_to_strings helper function before decorator chain to ensure CLI decorators are correctly applied to build() function. Addresses decorator misplacement issue identified in code review. Signed-off-by: Nikhil Ghosh <[email protected]>

ray-gardener bot added serve Ray Serve Related Issue llm labels Nov 11, 2025

nrghosh changed the title ~~[serve.llm] Fix serve build YAML serialization for AggregationFunction enum~~ [serve][llm] Fix serve build YAML serialization for AggregationFunction enum Nov 13, 2025

gemini-code-assist bot reviewed Nov 13, 2025

View reviewed changes

cursor bot reviewed Nov 13, 2025

View reviewed changes

python/ray/serve/scripts.py Show resolved Hide resolved

gemini-code-assist bot reviewed Nov 14, 2025

View reviewed changes

nrghosh added 2 commits November 13, 2025 16:08

Add type hints to enum_representer for consistency

3ecfbb8

Add type hints to match the pattern used in str_presenter function for better type safety and code consistency. Signed-off-by: Nikhil Ghosh <[email protected]>

gemini-code-assist bot reviewed Nov 14, 2025

View reviewed changes

python/ray/serve/scripts.py Outdated Show resolved Hide resolved

Update python/ray/serve/scripts.py

3d836a4

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Nikhil G <[email protected]>

nrghosh requested a review from a team November 14, 2025 00:16

nrghosh added the go add ONLY when ready to merge, run all tests label Nov 14, 2025

nrghosh and others added 2 commits November 13, 2025 16:34

docstring + safely handle all Enum types

73af9f9

Signed-off-by: Nikhil Ghosh <[email protected]>

Merge branch 'master' into nrghosh/serve-build-yaml-enum-fix

d9231d8

SekiroRong mentioned this pull request Nov 17, 2025

[Serve] serve build fails with RepresenterError when AutoscalingConfig is used #58645

Closed

nrghosh force-pushed the nrghosh/serve-build-yaml-enum-fix branch 2 times, most recently from 2a64295 to e4f91cf Compare November 17, 2025 22:56

abrarsheikh reviewed Nov 17, 2025

View reviewed changes

Add tests for the CLI build command

768e6df

Signed-off-by: Nikhil Ghosh <[email protected]>

nrghosh force-pushed the nrghosh/serve-build-yaml-enum-fix branch from e4f91cf to 768e6df Compare November 18, 2025 00:04

cursor bot reviewed Nov 18, 2025

View reviewed changes

python/ray/serve/scripts.py Outdated Show resolved Hide resolved

nrghosh added 2 commits November 17, 2025 16:37

wip - fix test

afe510f

Signed-off-by: Nikhil Ghosh <[email protected]>

wip

9c8798a

- Fix: Register Enum representer only on ServeDeploySchemaDumper to avoid global side effects - Also updated serve config and serve status to use the custom dumper. Signed-off-by: Nikhil Ghosh <[email protected]>

nrghosh requested a review from abrarsheikh November 18, 2025 23:06

abrarsheikh approved these changes Nov 18, 2025

View reviewed changes

python/ray/serve/scripts.py Outdated Show resolved Hide resolved

nrghosh and others added 3 commits November 18, 2025 18:52

wip- Register str representer directly on ServeDeploySchemaDumper

0fc9de0

Signed-off-by: Nikhil Ghosh <[email protected]>

Merge branch 'master' into nrghosh/serve-build-yaml-enum-fix

a7704bb

Merge branch 'master' into nrghosh/serve-build-yaml-enum-fix

adb6fea

nrghosh commented Nov 19, 2025

View reviewed changes

Merge branch 'master' into nrghosh/serve-build-yaml-enum-fix

25dff83

abrarsheikh enabled auto-merge (squash) November 19, 2025 20:43

abrarsheikh merged commit 5c49378 into ray-project:master Nov 19, 2025
7 checks passed

		super().write_line_break()


		def enum_representer(dumper, data):

	def enum_representer(dumper, data):
	def enum_representer(dumper: yaml.Dumper, data: Enum):

		yaml_str = yaml.safe_dump(config_dict, sort_keys=False)
		parsed = yaml.safe_load(yaml_str)

Conversation

nrghosh commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

tgowda1996 left a comment • 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!

nrghosh commented Nov 13, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

nrghosh commented Nov 14, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

nrghosh commented Nov 14, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

nrghosh commented Nov 14, 2025

Uh oh!

abrarsheikh commented Nov 17, 2025

Uh oh!

nrghosh commented Nov 17, 2025

Uh oh!

abrarsheikh Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

nrghosh Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

abrarsheikh Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

nrghosh Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nrghosh left a comment • 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nrghosh commented Nov 10, 2025 •

edited

Loading

tgowda1996 left a comment •

edited

Loading

nrghosh left a comment •

edited

Loading