Skip to content

Commit

Permalink
Relax protobuf<5.0.0 restriction (tensorflow#6888)
Browse files Browse the repository at this point in the history
Instead of restricting `protobuf<5.0.0` due to the removal to
`including_default_value_fields` kwarg in `json_format.MessageToJson`
(replaced by `always_print_fields_with_nopresence`), we can instead
detect the currently installed protobuf version and use the appropriate
kwarg.

Contributes to tensorflow#6808 and should resolve tensorflow#6887.
  • Loading branch information
groszewn committed Aug 13, 2024
1 parent 120d155 commit 9570874
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 23 deletions.
8 changes: 6 additions & 2 deletions tensorboard/pip_package/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@ absl-py >= 0.4
grpcio >= 1.48.2
markdown >= 2.6.8
numpy >= 1.12.0
# NOTE: The packaging dependency was introduced in order to compare installed
# package versions and conditionally use different supported kwargs
# (specifically the protobuf dependency). If we restrict protobuf >= 5.0.0 we
# can get rid of the packaging dependency.
packaging
# NOTE: this version must be >= the protoc version in our WORKSPACE file.
# At the same time, any constraints we specify here must allow at least some
# version to be installed that is also compatible with TensorFlow's constraints:
# https://github.com/tensorflow/tensorflow/blob/25adc4fccb4b0bb5a933eba1d246380e7b87d7f7/tensorflow/tools/pip_package/setup.py#L101
# 4.24.0 had an issue that broke our tests, so we should avoid that release:
# https://github.com/protocolbuffers/protobuf/issues/13485
# 5.26.0 introduced a breaking change, so we restricted it for now. See issue #6808 for details.
protobuf >= 3.19.6, != 4.24.0, < 5.0.0
protobuf >= 3.19.6, != 4.24.0
setuptools >= 41.0.0 # Note: provides pkg_resources as well as setuptools
# A dependency of our vendored packages. This lower bound has not been correctly
# vetted, but I wanted to be the least restrictive we can, since it's not a new
Expand Down
22 changes: 21 additions & 1 deletion tensorboard/plugin_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
# ==============================================================================
"""Provides utilities that may be especially useful to plugins."""


from google.protobuf import json_format
from importlib import metadata
from packaging import version
import threading

from bleach.sanitizer import Cleaner
Expand Down Expand Up @@ -193,6 +195,24 @@ def experiment_id(environ):
return environ.get(_experiment_id.WSGI_ENVIRON_KEY, "")


def proto_to_json(proto):
"""Utility method to convert proto to JSON, accounting for different version support.
Args:
proto: The proto to convert to JSON.
"""
current_version = metadata.version("protobuf")
if version.parse(current_version) >= version.parse("5.0.0"):
return json_format.MessageToJson(
proto,
always_print_fields_with_no_presence=True,
)
return json_format.MessageToJson(
proto,
including_default_value_fields=True,
)


class _MetadataVersionChecker:
"""TensorBoard-internal utility for warning when data is too new.
Expand Down
7 changes: 1 addition & 6 deletions tensorboard/plugins/custom_scalar/custom_scalars_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,8 @@
this plugin.
"""


import re

from google.protobuf import json_format

from werkzeug import wrappers

from tensorboard import plugin_util
Expand Down Expand Up @@ -318,9 +315,7 @@ def layout_impl(self, ctx, experiment):
title_to_category[category.title] = category

if merged_layout:
return json_format.MessageToJson(
merged_layout, including_default_value_fields=True
)
return plugin_util.proto_to_json(merged_layout)
else:
# No layout was found.
return {}
30 changes: 16 additions & 14 deletions tensorboard/plugins/hparams/hparams_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@
this plugin.
"""


import json


import werkzeug
from werkzeug import wrappers

Expand Down Expand Up @@ -116,14 +114,16 @@ def get_experiment_route(self, request):
request_proto = _parse_request_argument(
request, api_pb2.GetExperimentRequest
)
response_proto = get_experiment.Handler(
ctx,
self._context,
experiment_id,
request_proto,
).run()
response = plugin_util.proto_to_json(response_proto)
return http_util.Respond(
request,
json_format.MessageToJson(
get_experiment.Handler(
ctx, self._context, experiment_id, request_proto
).run(),
including_default_value_fields=True,
),
response,
"application/json",
)
except error.HParamsError as e:
Expand All @@ -139,14 +139,16 @@ def list_session_groups_route(self, request):
request_proto = _parse_request_argument(
request, api_pb2.ListSessionGroupsRequest
)
response_proto = list_session_groups.Handler(
ctx,
self._context,
experiment_id,
request_proto,
).run()
response = plugin_util.proto_to_json(response_proto)
return http_util.Respond(
request,
json_format.MessageToJson(
list_session_groups.Handler(
ctx, self._context, experiment_id, request_proto
).run(),
including_default_value_fields=True,
),
response,
"application/json",
)
except error.HParamsError as e:
Expand Down

0 comments on commit 9570874

Please sign in to comment.