From 5edcb74de8174a3d31f365ccb8b47b7427200979 Mon Sep 17 00:00:00 2001 From: Nick Groszewski Date: Wed, 7 Aug 2024 13:50:52 +0000 Subject: [PATCH] Relax `protobuf<5.0.0` restriction 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 #6808 and should resolve #6887. --- tensorboard/pip_package/requirements.txt | 8 +++- tensorboard/plugin_util.py | 14 +++++- tensorboard/plugin_util_test.py | 20 +++++++++ .../custom_scalar/custom_scalars_plugin.py | 12 +++-- tensorboard/plugins/hparams/hparams_plugin.py | 44 +++++++++++++------ 5 files changed, 77 insertions(+), 21 deletions(-) diff --git a/tensorboard/pip_package/requirements.txt b/tensorboard/pip_package/requirements.txt index a19090f321a..0cc39ff1ff6 100644 --- a/tensorboard/pip_package/requirements.txt +++ b/tensorboard/pip_package/requirements.txt @@ -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 diff --git a/tensorboard/plugin_util.py b/tensorboard/plugin_util.py index d589cc26ca4..43d939a3d22 100644 --- a/tensorboard/plugin_util.py +++ b/tensorboard/plugin_util.py @@ -14,7 +14,8 @@ # ============================================================================== """Provides utilities that may be especially useful to plugins.""" - +from importlib import metadata +from packaging import version import threading from bleach.sanitizer import Cleaner @@ -193,6 +194,17 @@ def experiment_id(environ): return environ.get(_experiment_id.WSGI_ENVIRON_KEY, "") +def package_version_greater_than_or_equal(package_name, version_string): + """Compare the package name is greater than or equal to the provided version. + + Args: + package_name: The package name to be checked. + version: The version string to compare the package against. + """ + current_version = metadata.version(package_name) + return version.parse(current_version) >= version.parse(version_string) + + class _MetadataVersionChecker: """TensorBoard-internal utility for warning when data is too new. diff --git a/tensorboard/plugin_util_test.py b/tensorboard/plugin_util_test.py index 68a5a916169..ac2172fae45 100644 --- a/tensorboard/plugin_util_test.py +++ b/tensorboard/plugin_util_test.py @@ -14,6 +14,7 @@ import textwrap +from unittest import mock from tensorboard import context @@ -229,5 +230,24 @@ def test_present(self): self.assertEqual(plugin_util.experiment_id(environ), "123") +class PackageVersionTest(tb_test.TestCase): + """Tests for `plugin_util.package_version_greater_than_or_equal`.""" + + def test_package_version_greater_than(self): + with mock.patch("importlib.metadata.version") as mock_version: + mock_version.return_value = "5.0.1" + self.assertTrue(plugin_util.package_version_greater_than_or_equal("test", "5.0.0")) + + def test_package_version_equal_to(self): + with mock.patch("importlib.metadata.version") as mock_version: + mock_version.return_value = "5.0.0" + self.assertTrue(plugin_util.package_version_greater_than_or_equal("test", "5.0.0")) + + def test_package_version_less_than(self): + with mock.patch("importlib.metadata.version") as mock_version: + mock_version.return_value = "4.9.9" + self.assertFalse(plugin_util.package_version_greater_than_or_equal("test", "5.0.0")) + + if __name__ == "__main__": tb_test.main() diff --git a/tensorboard/plugins/custom_scalar/custom_scalars_plugin.py b/tensorboard/plugins/custom_scalar/custom_scalars_plugin.py index 697c36f969a..27943a6c504 100644 --- a/tensorboard/plugins/custom_scalar/custom_scalars_plugin.py +++ b/tensorboard/plugins/custom_scalar/custom_scalars_plugin.py @@ -21,7 +21,6 @@ this plugin. """ - import re from google.protobuf import json_format @@ -318,9 +317,14 @@ 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 - ) + if plugin_util.package_version_greater_than_or_equal("protobuf", "5.0"): + response = json_format.MessageToJson( + merged_layout, always_print_fields_with_no_presence=True, + ) + else: + response = json_format.MessageToJson( + merged_layout, including_default_value_fields=True, + ) else: # No layout was found. return {} diff --git a/tensorboard/plugins/hparams/hparams_plugin.py b/tensorboard/plugins/hparams/hparams_plugin.py index d11e048099f..43969334574 100644 --- a/tensorboard/plugins/hparams/hparams_plugin.py +++ b/tensorboard/plugins/hparams/hparams_plugin.py @@ -18,10 +18,8 @@ this plugin. """ - import json - import werkzeug from werkzeug import wrappers @@ -116,14 +114,23 @@ 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() + if plugin_util.package_version_greater_than_or_equal("protobuf", "5.0.0"): + response = json_format.MessageToJson( + response_proto, always_print_fields_with_no_presence=True, + ) + else: + response = json_format.MessageToJson( + response_proto, including_default_value_fields=True, + ) 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: @@ -139,14 +146,23 @@ 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() + if plugin_util.package_version_greater_than_or_equal("protobuf", "5.0.0"): + response = json_format.MessageToJson( + response_proto, always_print_fields_with_no_presence=True, + ) + else: + response = json_format.MessageToJson( + response_proto, including_default_value_fields=True, + ) 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: