Skip to content

Commit

Permalink
[core] Fix protobuf breaking change by adding a compat layer. (#43172) (
Browse files Browse the repository at this point in the history
#43188)

Latest postmerge failure linux://python/ray/tests:test_output turns out to be from a recent protobuf pre-release change. It changed an argument name which breaks our event logger. This PR introduces a compatibility layer to support both args.

Also re-enabled core minimal tests in premerge.

Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
  • Loading branch information
rynewang committed Feb 15, 2024
1 parent 2613d7d commit 62655e1
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 23 deletions.
4 changes: 3 additions & 1 deletion .buildkite/core.rayci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,9 @@ steps:
job_env: forge-aarch64

- label: ":ray: core: minimal tests {{matrix}}"
tags: python
tags:
- python
- oss
instance_type: medium
commands:
# validate minimal installation
Expand Down
2 changes: 1 addition & 1 deletion dashboard/modules/actor/actor_head.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def actor_table_data_to_dict(message):
"parentTaskId",
"sourceActorId",
},
including_default_value_fields=True,
always_print_fields_with_no_presence=True,
)
# The complete schema for actor table is here:
# src/ray/protobuf/gcs.proto
Expand Down
2 changes: 1 addition & 1 deletion dashboard/modules/actor/tests/test_actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def actor_table_data_to_dict(message):
"sourceActorId",
"placementGroupId",
},
including_default_value_fields=False,
always_print_fields_with_no_presence=False,
)

non_state_keys = ("actorId", "jobId")
Expand Down
4 changes: 2 additions & 2 deletions dashboard/modules/node/node_head.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

def gcs_node_info_to_dict(message):
return dashboard_utils.message_to_dict(
message, {"nodeId"}, including_default_value_fields=True
message, {"nodeId"}, always_print_fields_with_no_presence=True
)


Expand Down Expand Up @@ -80,7 +80,7 @@ def node_stats_to_dict(message):
result = dashboard_utils.message_to_dict(message, decode_keys)
result["coreWorkersStats"] = [
dashboard_utils.message_to_dict(
m, decode_keys, including_default_value_fields=True
m, decode_keys, always_print_fields_with_no_presence=True
)
for m in core_workers_stats
]
Expand Down
11 changes: 6 additions & 5 deletions dashboard/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import aiosignal # noqa: F401

from google.protobuf.json_format import MessageToDict
import ray._private.protobuf_compat
from frozenlist import FrozenList # noqa: F401

from ray._private.utils import binary_to_hex, check_dashboard_dependencies_installed
Expand Down Expand Up @@ -217,12 +217,13 @@ def _decode_keys(d):
d[k] = v
return d

d = ray._private.protobuf_compat.message_to_dict(
message, use_integers_for_enums=False, **kwargs
)
if decode_keys:
return _decode_keys(
MessageToDict(message, use_integers_for_enums=False, **kwargs)
)
return _decode_keys(d)
else:
return MessageToDict(message, use_integers_for_enums=False, **kwargs)
return d


class SignalManager:
Expand Down
7 changes: 4 additions & 3 deletions python/ray/_private/event/event_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
from typing import Dict, Optional
from datetime import datetime

from google.protobuf.json_format import MessageToDict, Parse
from google.protobuf.json_format import Parse

from ray.core.generated.event_pb2 import Event
from ray._private.protobuf_compat import message_to_dict

global_logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -91,9 +92,9 @@ def _emit(self, severity: Event.Severity, message: str, **kwargs):

self.logger.info(
json.dumps(
MessageToDict(
message_to_dict(
event,
including_default_value_fields=True,
always_print_fields_with_no_presence=True,
preserving_proto_field_name=True,
use_integers_for_enums=False,
)
Expand Down
41 changes: 41 additions & 0 deletions python/ray/_private/protobuf_compat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
from google.protobuf.json_format import MessageToDict
import inspect

"""
This module provides a compatibility layer for different versions of the protobuf
library.
"""


def rename_always_print_fields_with_no_presence(kwargs):
"""
Protobuf version 5.26.0rc2 renamed argument for `MessageToDict`:
`including_default_value_fields` -> `always_print_fields_with_no_presence`.
See https://github.com/protocolbuffers/protobuf/commit/06e7caba58ede0220b110b89d08f329e5f8a7537#diff-8de817c14d6a087981503c9aea38730b1b3e98f4e306db5ff9d525c7c304f234L129 # noqa: E501
We choose to always use the new argument name. If user used the old arg, we raise an
error.
If protobuf does not have the new arg name but have the old arg name, we rename our
arg to the old one.
"""
old_arg_name = "including_default_value_fields"
new_arg_name = "always_print_fields_with_no_presence"
if old_arg_name in kwargs:
raise ValueError(f"{old_arg_name} is deprecated, please use {new_arg_name}")
if new_arg_name not in kwargs:
return kwargs

params = inspect.signature(MessageToDict).parameters
if new_arg_name in params:
return kwargs
if old_arg_name in params:
kwargs[old_arg_name] = kwargs.pop(new_arg_name)
return kwargs
# Neither args are in the signature, do nothing.
return kwargs


def message_to_dict(*args, **kwargs):
kwargs = rename_always_print_fields_with_no_presence(kwargs)
return MessageToDict(*args, **kwargs)
4 changes: 2 additions & 2 deletions python/ray/_private/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from collections import defaultdict
from typing import Set

from google.protobuf.json_format import MessageToDict
from ray._private.protobuf_compat import message_to_dict

import ray
from ray._private.client_mode_hook import client_mode_hook
Expand Down Expand Up @@ -346,7 +346,7 @@ def get_strategy(strategy):
"bundles": {
# The value here is needs to be dictionarified
# otherwise, the payload becomes unserializable.
bundle.bundle_id.bundle_index: MessageToDict(bundle)["unitResources"]
bundle.bundle_id.bundle_index: message_to_dict(bundle)["unitResources"]
for bundle in placement_group_info.bundles
},
"bundles_to_node_id": {
Expand Down
7 changes: 4 additions & 3 deletions python/ray/autoscaler/v2/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
from enum import Enum
from typing import Dict, List, Optional, Tuple

from google.protobuf.json_format import MessageToDict

from ray._private.protobuf_compat import message_to_dict
from ray.autoscaler._private.resource_demand_scheduler import UtilizationScore
from ray.autoscaler.v2.schema import NodeType
from ray.autoscaler.v2.utils import is_pending, resource_requests_by_count
Expand Down Expand Up @@ -245,7 +244,9 @@ def __repr__(self) -> str:
available_resources=self.available_resources,
labels=self.labels,
launch_reason=self.launch_reason,
sched_requests="|".join(str(MessageToDict(r)) for r in self.sched_requests),
sched_requests="|".join(
str(message_to_dict(r)) for r in self.sched_requests
),
)


Expand Down
7 changes: 3 additions & 4 deletions python/ray/serve/_private/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
import json
from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union

from google.protobuf.json_format import MessageToDict

from ray import cloudpickle
from ray._private import ray_option_utils
from ray._private.protobuf_compat import message_to_dict
from ray._private.pydantic_compat import (
BaseModel,
Field,
Expand Down Expand Up @@ -199,9 +198,9 @@ def to_proto_bytes(self):

@classmethod
def from_proto(cls, proto: DeploymentConfigProto):
data = MessageToDict(
data = message_to_dict(
proto,
including_default_value_fields=True,
always_print_fields_with_no_presence=True,
preserving_proto_field_name=True,
use_integers_for_enums=True,
)
Expand Down
2 changes: 1 addition & 1 deletion python/ray/util/state/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,7 @@ def protobuf_message_to_dict(
return dashboard_utils.message_to_dict(
message,
fields_to_decode,
including_default_value_fields=True,
always_print_fields_with_no_presence=True,
preserving_proto_field_name=preserving_proto_field_name,
)

Expand Down

0 comments on commit 62655e1

Please sign in to comment.