diff --git a/tensorboard/uploader/proto/server_info.proto b/tensorboard/uploader/proto/server_info.proto index e2d43e79fc..b221124cd1 100644 --- a/tensorboard/uploader/proto/server_info.proto +++ b/tensorboard/uploader/proto/server_info.proto @@ -8,21 +8,11 @@ package tensorboard.service; message ServerInfoRequest { // Client-side TensorBoard version, per `tensorboard.version.VERSION`. string version = 1; - // Information about the plugins for which the client wishes to upload data. - // - // If specified then the list of plugins will be confirmed by the server and - // echoed in the PluginControl.allowed_plugins field. Otherwise the server - // will return the default set of plugins it supports. - // - // If one of the plugins is not supported by the server then it will respond - // with compatibility verdict VERDICT_ERROR. - PluginSpecification plugin_specification = 2; } message ServerInfoResponse { - // Primary bottom-line: is the server compatible with the client, can it - // serve its request, and is there anything that the end user should be - // aware of? + // Primary bottom-line: is the server compatible with the client, and is + // there anything that the end user should be aware of? Compatibility compatibility = 1; // Identifier for a gRPC server providing the `TensorBoardExporterService` and // `TensorBoardWriterService` services (under the `tensorboard.service` proto @@ -30,15 +20,19 @@ message ServerInfoResponse { ApiServer api_server = 2; // How to generate URLs to experiment pages. ExperimentUrlFormat url_format = 3; - // Information about the plugins for which data should be uploaded. + // For which plugins should we upload data? (Even if the uploader is + // structurally capable of uploading data from many plugins, we only actually + // upload data that can be currently displayed in TensorBoard.dev. Otherwise, + // users may be surprised to see that experiments that they uploaded a while + // ago and have since shared or published now have extra information that + // they didn't realize had been uploaded.) // - // If PluginSpecification.requested_plugins is specified then - // that list of plugins will be confirmed by the server and echoed in the - // the response. Otherwise the server will return the default set of - // plugins it supports. + // The client may always choose to upload less data than is permitted by this + // field: e.g., if the end user specifies not to upload data for a given + // plugin, or the client does not yet support uploading some kind of data. // - // The client should only upload data for the plugins in the response even - // if it is capable of uploading more data. + // If this field is omitted, there are no upfront restrictions on what the + // client may send. PluginControl plugin_control = 4; } @@ -80,15 +74,8 @@ message ExperimentUrlFormat { string id_placeholder = 2; } -message PluginSpecification { - // Plugins for which the client wishes to upload data. These are plugin names - // as stored in the the `SummaryMetadata.plugin_data.plugin_name` proto - // field. - repeated string upload_plugins = 2; -} - message PluginControl { - // Plugins for which data should be uploaded. These are plugin names as + // Only send data from plugins with these names. These are plugin names as // stored in the the `SummaryMetadata.plugin_data.plugin_name` proto field. repeated string allowed_plugins = 1; } diff --git a/tensorboard/uploader/server_info.py b/tensorboard/uploader/server_info.py index 6416b694e0..f031830dcc 100644 --- a/tensorboard/uploader/server_info.py +++ b/tensorboard/uploader/server_info.py @@ -21,7 +21,6 @@ from google.protobuf import message import requests -from absl import logging from tensorboard import version from tensorboard.plugins.scalar import metadata as scalars_metadata from tensorboard.uploader.proto import server_info_pb2 @@ -31,31 +30,19 @@ _REQUEST_TIMEOUT_SECONDS = 10 -def _server_info_request(upload_plugins): - """Generates a ServerInfoRequest - - Args: - upload_plugins: List of plugin names requested by the user and to be - verified by the server. - - Returns: - A `server_info_pb2.ServerInfoRequest` message. - """ +def _server_info_request(): request = server_info_pb2.ServerInfoRequest() request.version = version.VERSION - request.plugin_specification.upload_plugins[:] = upload_plugins return request -def fetch_server_info(origin, upload_plugins): +def fetch_server_info(origin): """Fetches server info from a remote server. Args: origin: The server with which to communicate. Should be a string like "https://tensorboard.dev", including protocol, host, and (if needed) port. - upload_plugins: List of plugins names requested by the user and to be - verified by the server. Returns: A `server_info_pb2.ServerInfoResponse` message. @@ -65,9 +52,7 @@ def fetch_server_info(origin, upload_plugins): communicate with the remote server. """ endpoint = "%s/api/uploader" % origin - server_info_request = _server_info_request(upload_plugins) - post_body = server_info_request.SerializeToString() - logging.info("Requested server info: <%r>", server_info_request) + post_body = _server_info_request().SerializeToString() try: response = requests.post( endpoint, @@ -90,15 +75,13 @@ def fetch_server_info(origin, upload_plugins): ) -def create_server_info(frontend_origin, api_endpoint, upload_plugins): +def create_server_info(frontend_origin, api_endpoint): """Manually creates server info given a frontend and backend. Args: frontend_origin: The origin of the TensorBoard.dev frontend, like "https://tensorboard.dev" or "http://localhost:8000". api_endpoint: As to `server_info_pb2.ApiServer.endpoint`. - upload_plugins: List of plugin names requested by the user and to be - verified by the server. Returns: A `server_info_pb2.ServerInfoResponse` message. @@ -112,7 +95,6 @@ def create_server_info(frontend_origin, api_endpoint, upload_plugins): placeholder = "{%s}" % placeholder url_format.template = "%s/experiment/%s/" % (frontend_origin, placeholder) url_format.id_placeholder = placeholder - result.plugin_control.allowed_plugins[:] = upload_plugins return result diff --git a/tensorboard/uploader/server_info_test.py b/tensorboard/uploader/server_info_test.py index e714486b46..129bf36b2a 100644 --- a/tensorboard/uploader/server_info_test.py +++ b/tensorboard/uploader/server_info_test.py @@ -70,31 +70,12 @@ def app(request): body = request.get_data() request_pb = server_info_pb2.ServerInfoRequest.FromString(body) self.assertEqual(request_pb.version, version.VERSION) - self.assertEqual(request_pb.plugin_specification.upload_plugins, []) return wrappers.BaseResponse(expected_result.SerializeToString()) origin = self._start_server(app) - result = server_info.fetch_server_info(origin, []) + result = server_info.fetch_server_info(origin) self.assertEqual(result, expected_result) - def test_fetches_with_plugins(self): - @wrappers.BaseRequest.application - def app(request): - body = request.get_data() - request_pb = server_info_pb2.ServerInfoRequest.FromString(body) - self.assertEqual(request_pb.version, version.VERSION) - self.assertEqual( - request_pb.plugin_specification.upload_plugins, - ["plugin1", "plugin2"], - ) - return wrappers.BaseResponse( - server_info_pb2.ServerInfoResponse().SerializeToString() - ) - - origin = self._start_server(app) - result = server_info.fetch_server_info(origin, ["plugin1", "plugin2"]) - self.assertIsNotNone(result) - def test_econnrefused(self): (family, localhost) = _localhost() s = socket.socket(family) @@ -102,7 +83,7 @@ def test_econnrefused(self): self.addCleanup(s.close) port = s.getsockname()[1] with self.assertRaises(server_info.CommunicationError) as cm: - server_info.fetch_server_info("http://localhost:%d" % port, []) + server_info.fetch_server_info("http://localhost:%d" % port) msg = str(cm.exception) self.assertIn("Failed to connect to backend", msg) if os.name != "nt": @@ -116,7 +97,7 @@ def app(request): origin = self._start_server(app) with self.assertRaises(server_info.CommunicationError) as cm: - server_info.fetch_server_info(origin, []) + server_info.fetch_server_info(origin) msg = str(cm.exception) self.assertIn("Non-OK status from backend (502 Bad Gateway)", msg) self.assertIn("very sad", msg) @@ -129,7 +110,7 @@ def app(request): origin = self._start_server(app) with self.assertRaises(server_info.CommunicationError) as cm: - server_info.fetch_server_info(origin, []) + server_info.fetch_server_info(origin) msg = str(cm.exception) self.assertIn("Corrupt response from backend", msg) self.assertIn("an unlikely proto", msg) @@ -142,7 +123,7 @@ def app(request): return wrappers.BaseResponse(result.SerializeToString()) origin = self._start_server(app) - result = server_info.fetch_server_info(origin, []) + result = server_info.fetch_server_info(origin) expected_user_agent = "tensorboard/%s" % version.VERSION self.assertEqual(result.compatibility.details, expected_user_agent) @@ -150,10 +131,10 @@ def app(request): class CreateServerInfoTest(tb_test.TestCase): """Tests for `create_server_info`.""" - def test_response(self): + def test(self): frontend = "http://localhost:8080" backend = "localhost:10000" - result = server_info.create_server_info(frontend, backend, []) + result = server_info.create_server_info(frontend, backend) expected_compatibility = server_info_pb2.Compatibility() expected_compatibility.verdict = server_info_pb2.VERDICT_OK @@ -171,19 +152,6 @@ def test_response(self): expected_url = "http://localhost:8080/experiment/123/" self.assertEqual(actual_url, expected_url) - self.assertEqual(result.plugin_control.allowed_plugins, []) - - def test_response_with_plugins(self): - frontend = "http://localhost:8080" - backend = "localhost:10000" - result = server_info.create_server_info( - frontend, backend, ["plugin1", "plugin2"] - ) - - self.assertEqual( - result.plugin_control.allowed_plugins, ["plugin1", "plugin2"] - ) - class ExperimentUrlTest(tb_test.TestCase): """Tests for `experiment_url`.""" diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py index 75deb6366f..546fca3258 100644 --- a/tensorboard/uploader/uploader_main.py +++ b/tensorboard/uploader/uploader_main.py @@ -159,16 +159,6 @@ def _define_flags(parser): help="Experiment description. Markdown format. Max 600 characters.", ) - upload.add_argument( - "--plugins", - type=str, - nargs="*", - default=[], - help="List of plugins for which data should be uploaded. If " - "unspecified then data will be uploaded for all plugins supported by " - "the server.", - ) - update_metadata = subparsers.add_parser( "update-metadata", help="change the name, description, or other user " @@ -744,10 +734,8 @@ def _get_intent(flags): def _get_server_info(flags): origin = flags.origin or _DEFAULT_ORIGIN if flags.api_endpoint and not flags.origin: - return server_info_lib.create_server_info( - origin, flags.api_endpoint, flags.plugins - ) - server_info = server_info_lib.fetch_server_info(origin, flags.plugins) + return server_info_lib.create_server_info(origin, flags.api_endpoint) + server_info = server_info_lib.fetch_server_info(origin) # Override with any API server explicitly specified on the command # line, but only if the server accepted our initial handshake. if flags.api_endpoint and server_info.api_server.endpoint: