From 45ea650816b6c94c9860de369a8ad2e2eb124660 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 09:56:13 -0500
Subject: [PATCH 01/12] Adds name and description to the output of "list"
 subcommand

---
 tensorboard/uploader/uploader_main.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index 57c406837a..f162a9ae82 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -409,6 +409,8 @@ def execute(self, server_info, channel):
             url = server_info_lib.experiment_url(server_info, experiment_id)
             print(url)
             data = [
+                ("Name", experiment.name or "[No Name]"),
+                ("Description", experiment.description or "[No Description]"),
                 ("Id", experiment.experiment_id),
                 ("Created", util.format_time(experiment.create_time)),
                 ("Updated", util.format_time(experiment.update_time)),
@@ -417,7 +419,7 @@ def execute(self, server_info, channel):
                 ("Tags", str(experiment.num_tags)),
             ]
             for (name, value) in data:
-                print("\t%s %s" % (name.ljust(10), value))
+                print("\t%s %s" % (name.ljust(11), value))
         sys.stdout.flush()
         if not count:
             sys.stderr.write(

From cd79b29e179de46eaeb978f8fafa1700c2f9de70 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 11:21:27 -0500
Subject: [PATCH 02/12] Adds name and description as upload options to the
 uploader

---
 tensorboard/uploader/uploader.py      | 17 +++++++--
 tensorboard/uploader/uploader_main.py | 50 +++++++++++++++++++++++++--
 tensorboard/uploader/uploader_test.py | 17 +++++++++
 3 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/tensorboard/uploader/uploader.py b/tensorboard/uploader/uploader.py
index 8dcd401a26..58d24e47dd 100644
--- a/tensorboard/uploader/uploader.py
+++ b/tensorboard/uploader/uploader.py
@@ -64,7 +64,14 @@
 class TensorBoardUploader(object):
     """Uploads a TensorBoard logdir to TensorBoard.dev."""
 
-    def __init__(self, writer_client, logdir, rpc_rate_limiter=None):
+    def __init__(
+        self,
+        writer_client,
+        logdir,
+        rpc_rate_limiter=None,
+        name=None,
+        description=None,
+    ):
         """Constructs a TensorBoardUploader.
 
         Args:
@@ -77,9 +84,13 @@ def __init__(self, writer_client, logdir, rpc_rate_limiter=None):
             of chunks.  Note the chunk stream is internally rate-limited by
             backpressure from the server, so it is not a concern that we do not
             explicitly rate-limit within the stream here.
+          name: String name to assign to the experiment.
+          description: String description to assign to the experiment.
         """
         self._api = writer_client
         self._logdir = logdir
+        self._name = name
+        self._description = description
         self._request_sender = None
         if rpc_rate_limiter is None:
             self._rpc_rate_limiter = util.RateLimiter(
@@ -103,7 +114,9 @@ def __init__(self, writer_client, logdir, rpc_rate_limiter=None):
     def create_experiment(self):
         """Creates an Experiment for this upload session and returns the ID."""
         logger.info("Creating experiment")
-        request = write_service_pb2.CreateExperimentRequest()
+        request = write_service_pb2.CreateExperimentRequest(
+            name=self._name, description=self._description
+        )
         response = grpc_util.call_with_retries(
             self._api.CreateExperiment, request
         )
diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index f162a9ae82..25c147f494 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -72,6 +72,12 @@
 _DEFAULT_ORIGIN = "https://tensorboard.dev"
 
 
+# Size limits for input fields not bounded at a wire level. "Chars" in this
+# context refers to Unicode code points as stipulated by https://aip.dev/210.
+_EXPERIMENT_NAME_MAX_CHARS = 100
+_EXPERIMENT_DESCRIPTION_MAX_CHARS = 600
+
+
 def _prompt_for_user_ack(intent):
     """Prompts for user consent, exiting the program if they decline."""
     body = intent.get_ack_message_body()
@@ -139,6 +145,18 @@ def _define_flags(parser):
         default=None,
         help="Directory containing the logs to process",
     )
+    upload.add_argument(
+        "--name",
+        type=str,
+        default=None,
+        help="Title of the experiment.  Max 100 characters.",
+    )
+    upload.add_argument(
+        "--description",
+        type=str,
+        default=None,
+        help="Experiment description. Markdown format.  Max 600 characters.",
+    )
 
     delete = subparsers.add_parser(
         "delete",
@@ -445,8 +463,10 @@ class _UploadIntent(_Intent):
         """
     )
 
-    def __init__(self, logdir):
+    def __init__(self, logdir, name=None, description=None):
         self.logdir = logdir
+        self.name = name
+        self.description = description
 
     def get_ack_message_body(self):
         return self._MESSAGE_TEMPLATE.format(logdir=self.logdir)
@@ -455,7 +475,27 @@ def execute(self, server_info, channel):
         api_client = write_service_pb2_grpc.TensorBoardWriterServiceStub(
             channel
         )
-        uploader = uploader_lib.TensorBoardUploader(api_client, self.logdir)
+        if self.name and len(self.name) > _EXPERIMENT_NAME_MAX_CHARS:
+            raise ValueError(
+                "Experiment name is too long.  Limit is "
+                f"{_EXPERIMENT_NAME_MAX_CHARS} characters.\n"
+                f"{repr(self.name)} was provided."
+            )
+        if (
+            self.description
+            and len(self.description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS
+        ):
+            raise ValueError(
+                "Experiment description is too long.  Limit is "
+                f"{_EXPERIMENT_DESCRIPTION_MAX_CHARS} characters.\n"
+                f"{repr(self.description)} was provided."
+            )
+        uploader = uploader_lib.TensorBoardUploader(
+            api_client,
+            self.logdir,
+            name=self.name,
+            description=self.description,
+        )
         experiment_id = uploader.create_experiment()
         url = server_info_lib.experiment_url(server_info, experiment_id)
         print(
@@ -543,7 +583,11 @@ def _get_intent(flags):
         raise base_plugin.FlagsError("Must specify subcommand (try --help).")
     if cmd == _SUBCOMMAND_KEY_UPLOAD:
         if flags.logdir:
-            return _UploadIntent(os.path.expanduser(flags.logdir))
+            return _UploadIntent(
+                os.path.expanduser(flags.logdir),
+                name=flags.name,
+                description=flags.description,
+            )
         else:
             raise base_plugin.FlagsError(
                 "Must specify directory to upload via `--logdir`."
diff --git a/tensorboard/uploader/uploader_test.py b/tensorboard/uploader/uploader_test.py
index a714370419..551e648e36 100644
--- a/tensorboard/uploader/uploader_test.py
+++ b/tensorboard/uploader/uploader_test.py
@@ -75,6 +75,23 @@ def test_create_experiment(self):
         eid = uploader.create_experiment()
         self.assertEqual(eid, "123")
 
+    def test_create_experiment_with_name_and_description(self):
+        logdir = "/logs/foo"
+        mock_client = _create_mock_client()
+        uploader_name = uploader_lib.TensorBoardUploader(
+            mock_client, logdir, name="name")
+        uploader_description = uploader_lib.TensorBoardUploader(
+            mock_client, logdir, description="description")
+        uploader_both = uploader_lib.TensorBoardUploader(
+            mock_client, logdir, name="name", description="description")
+        eid_name = uploader_name.create_experiment()
+        self.assertEqual(eid_name, "123")
+        eid_description = uploader_description.create_experiment()
+        self.assertEqual(eid_description, "123")
+        eid_both = uploader_both.create_experiment()
+        self.assertEqual(eid_both, "123")
+
+
     def test_start_uploading_without_create_experiment_fails(self):
         mock_client = _create_mock_client()
         uploader = uploader_lib.TensorBoardUploader(mock_client, "/logs/foo")

From 60cef9a90add07150d5cf2e3641332627226f438 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 12:30:51 -0500
Subject: [PATCH 03/12] add subcommand for update-metadata to change name and
 description

---
 tensorboard/uploader/uploader.py      |  45 ++++++++
 tensorboard/uploader/uploader_main.py | 145 +++++++++++++++++++++++---
 2 files changed, 175 insertions(+), 15 deletions(-)

diff --git a/tensorboard/uploader/uploader.py b/tensorboard/uploader/uploader.py
index 58d24e47dd..e3011cd72f 100644
--- a/tensorboard/uploader/uploader.py
+++ b/tensorboard/uploader/uploader.py
@@ -26,6 +26,7 @@
 import six
 
 from tensorboard.uploader.proto import write_service_pb2
+from tensorboard.uploader.proto import experiment_pb2
 from tensorboard.uploader import logdir_loader
 from tensorboard.uploader import util
 from tensorboard import data_compat
@@ -153,6 +154,50 @@ def _upload_once(self):
         self._request_sender.send_requests(run_to_events)
 
 
+def update_experiment_metadata(
+    writer_client, experiment_id, name=None, description=None
+):
+    """Modifies user data associated with an experiment.
+
+    Args:
+      writer_client: a TensorBoardWriterService stub instance
+      experiment_id: string ID of the experiment to modify
+      name: If provided, modifies name of experiment to this value.
+      description: If provided, modifies the description of the experiment to
+         this value
+
+    Raises:
+      ExperimentNotFoundError: If no such experiment exists.
+      PermissionDeniedError: If the user is not authorized to modify this
+        experiment.
+      RuntimeError: On unexpected failure.
+    """
+    logger.info("Modifying experiment %r", experiment_id)
+    if name is not None:
+        logger.info("Setting exp %r name to ", experiment_id, name)
+    if description is not None:
+        logger.info(
+            "Setting exp %r description to ", experiment_id, description
+        )
+    experiment = experiment_pb2.Experiment(
+        experiment_id=experiment_id, name=name, description=description
+    )
+    experiment_mask = experiment_pb2.ExperimentMask(
+        name=name is not None, description=description is not None
+    )
+    request = write_service_pb2.UpdateExperimentRequest(
+        experiment=experiment, experiment_mask=experiment_mask
+    )
+    try:
+        grpc_util.call_with_retries(writer_client.UpdateExperiment, request)
+    except grpc.RpcError as e:
+        if e.code() == grpc.StatusCode.NOT_FOUND:
+            raise ExperimentNotFoundError()
+        if e.code() == grpc.StatusCode.PERMISSION_DENIED:
+            raise PermissionDeniedError()
+        raise
+
+
 def delete_experiment(writer_client, experiment_id):
     """Permanently deletes an experiment and all of its contents.
 
diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index 25c147f494..9e72f36cfc 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -65,6 +65,7 @@
 _SUBCOMMAND_KEY_DELETE = "DELETE"
 _SUBCOMMAND_KEY_LIST = "LIST"
 _SUBCOMMAND_KEY_EXPORT = "EXPORT"
+_SUBCOMMAND_KEY_UPDATE_METADATA = "UPDATEMETADATA"
 _SUBCOMMAND_KEY_AUTH = "AUTH"
 _AUTH_SUBCOMMAND_FLAG = "_uploader__subcommand_auth"
 _AUTH_SUBCOMMAND_KEY_REVOKE = "REVOKE"
@@ -158,6 +159,34 @@ def _define_flags(parser):
         help="Experiment description. Markdown format.  Max 600 characters.",
     )
 
+    update_metadata = subparsers.add_parser(
+        "update-metadata",
+        help="change the name, description, or other user "
+        "metadata associated with an experiment.",
+    )
+    update_metadata.set_defaults(
+        **{_SUBCOMMAND_FLAG: _SUBCOMMAND_KEY_UPDATE_METADATA}
+    )
+    update_metadata.add_argument(
+        "--experiment_id",
+        metavar="EXPERIMENT_ID",
+        type=str,
+        default=None,
+        help="ID of the experiment on which to modify the metadata.",
+    )
+    update_metadata.add_argument(
+        "--name",
+        type=str,
+        default=None,
+        help="Title of the experiment.  Max 100 characters.",
+    )
+    update_metadata.add_argument(
+        "--description",
+        type=str,
+        default=None,
+        help="Experiment description. Markdown format.  Max 600 characters.",
+    )
+
     delete = subparsers.add_parser(
         "delete",
         help="permanently delete an experiment",
@@ -390,6 +419,68 @@ def execute(self, server_info, channel):
         print("Deleted experiment %s." % experiment_id)
 
 
+class _UpdateMetadataIntent(_Intent):
+    """The user intends to update the metadata for an experiment."""
+
+    _MESSAGE_TEMPLATE = textwrap.dedent(
+        u"""\
+        This will modify the metadata associated with the experiment on
+        https://tensorboard.dev with the following experiment ID:
+
+        {experiment_id}
+
+        You have chosen to modify an experiment. All experiments uploaded
+        to TensorBoard.dev are publicly visible. Do not upload sensitive
+        data.
+        """
+    )
+
+    def __init__(self, experiment_id, name=None, description=None):
+        self.experiment_id = experiment_id
+        self.name = name
+        self.description = description
+
+    def get_ack_message_body(self):
+        return self._MESSAGE_TEMPLATE.format(experiment_id=self.experiment_id)
+
+    def execute(self, server_info, channel):
+        api_client = write_service_pb2_grpc.TensorBoardWriterServiceStub(
+            channel
+        )
+        experiment_id = self.experiment_id
+        _raise_if_bad_experiment_name(self.name)
+        _raise_if_bad_experiment_description(self.description)
+        if not experiment_id:
+            raise base_plugin.FlagsError(
+                "Must specify a non-empty experiment ID to modify."
+            )
+        try:
+            uploader_lib.update_experiment_metadata(
+                api_client,
+                experiment_id,
+                name=self.name,
+                description=self.description,
+            )
+        except uploader_lib.ExperimentNotFoundError:
+            _die(
+                "No such experiment %s. Either it never existed or it has "
+                "already been deleted." % experiment_id
+            )
+        except uploader_lib.PermissionDeniedError:
+            _die(
+                "Cannot modify experiment %s because it is owned by a "
+                "different user." % experiment_id
+            )
+        except grpc.RpcError as e:
+            _die("Internal error modifying experiment: %s" % e)
+        print("Modified experiment %s." % experiment_id)
+        if self.name is not None:
+            print(f"Set name to {repr(self.name)}")
+        if self.description is not None:
+            print(f"Set description to {repr(self.description)}")
+
+
+
 class _ListIntent(_Intent):
     """The user intends to list all their experiments."""
 
@@ -448,6 +539,27 @@ def execute(self, server_info, channel):
         sys.stderr.flush()
 
 
+def _raise_if_bad_experiment_name(name):
+    if name and len(name) > _EXPERIMENT_NAME_MAX_CHARS:
+        raise ValueError(
+            "Experiment name is too long.  Limit is "
+            f"{_EXPERIMENT_NAME_MAX_CHARS} characters.\n"
+            f"{repr(name)} was provided."
+        )
+
+
+def _raise_if_bad_experiment_description(description):
+    if (
+        description
+        and len(description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS
+    ):
+        raise ValueError(
+            "Experiment description is too long.  Limit is "
+            f"{_EXPERIMENT_DESCRIPTION_MAX_CHARS} characters.\n"
+            f"{repr(description)} was provided."
+        )
+
+
 class _UploadIntent(_Intent):
     """The user intends to upload an experiment from the given logdir."""
 
@@ -475,21 +587,8 @@ def execute(self, server_info, channel):
         api_client = write_service_pb2_grpc.TensorBoardWriterServiceStub(
             channel
         )
-        if self.name and len(self.name) > _EXPERIMENT_NAME_MAX_CHARS:
-            raise ValueError(
-                "Experiment name is too long.  Limit is "
-                f"{_EXPERIMENT_NAME_MAX_CHARS} characters.\n"
-                f"{repr(self.name)} was provided."
-            )
-        if (
-            self.description
-            and len(self.description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS
-        ):
-            raise ValueError(
-                "Experiment description is too long.  Limit is "
-                f"{_EXPERIMENT_DESCRIPTION_MAX_CHARS} characters.\n"
-                f"{repr(self.description)} was provided."
-            )
+        _raise_if_bad_experiment_name(self.name)
+        _raise_if_bad_experiment_description(self.description)
         uploader = uploader_lib.TensorBoardUploader(
             api_client,
             self.logdir,
@@ -592,6 +691,22 @@ def _get_intent(flags):
             raise base_plugin.FlagsError(
                 "Must specify directory to upload via `--logdir`."
             )
+    if cmd == _SUBCOMMAND_KEY_UPDATE_METADATA:
+        if flags.experiment_id:
+            if flags.name is not None or flags.description is not None:
+                return _UpdateMetadataIntent(
+                    flags.experiment_id,
+                    name=flags.name,
+                    description=flags.description,
+                )
+            else:
+                raise base_plugin.FlagsError(
+                    "Must specify either `--name` or `--description`."
+                )
+        else:
+            raise base_plugin.FlagsError(
+                "Must specify experiment to modify via `--experiment_id`."
+            )
     elif cmd == _SUBCOMMAND_KEY_DELETE:
         if flags.experiment_id:
             return _DeleteExperimentIntent(flags.experiment_id)

From 4aeed3c8a072c1f7adff5b19cbb2ef9d468f5387 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 12:40:52 -0500
Subject: [PATCH 04/12] black

---
 tensorboard/uploader/uploader_main.py |  6 +-----
 tensorboard/uploader/uploader_test.py | 10 ++++++----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index 9e72f36cfc..d22dde9c0d 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -480,7 +480,6 @@ def execute(self, server_info, channel):
             print(f"Set description to {repr(self.description)}")
 
 
-
 class _ListIntent(_Intent):
     """The user intends to list all their experiments."""
 
@@ -549,10 +548,7 @@ def _raise_if_bad_experiment_name(name):
 
 
 def _raise_if_bad_experiment_description(description):
-    if (
-        description
-        and len(description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS
-    ):
+    if description and len(description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS:
         raise ValueError(
             "Experiment description is too long.  Limit is "
             f"{_EXPERIMENT_DESCRIPTION_MAX_CHARS} characters.\n"
diff --git a/tensorboard/uploader/uploader_test.py b/tensorboard/uploader/uploader_test.py
index 551e648e36..15b34aa2e4 100644
--- a/tensorboard/uploader/uploader_test.py
+++ b/tensorboard/uploader/uploader_test.py
@@ -79,11 +79,14 @@ def test_create_experiment_with_name_and_description(self):
         logdir = "/logs/foo"
         mock_client = _create_mock_client()
         uploader_name = uploader_lib.TensorBoardUploader(
-            mock_client, logdir, name="name")
+            mock_client, logdir, name="name"
+        )
         uploader_description = uploader_lib.TensorBoardUploader(
-            mock_client, logdir, description="description")
+            mock_client, logdir, description="description"
+        )
         uploader_both = uploader_lib.TensorBoardUploader(
-            mock_client, logdir, name="name", description="description")
+            mock_client, logdir, name="name", description="description"
+        )
         eid_name = uploader_name.create_experiment()
         self.assertEqual(eid_name, "123")
         eid_description = uploader_description.create_experiment()
@@ -91,7 +94,6 @@ def test_create_experiment_with_name_and_description(self):
         eid_both = uploader_both.create_experiment()
         self.assertEqual(eid_both, "123")
 
-
     def test_start_uploading_without_create_experiment_fails(self):
         mock_client = _create_mock_client()
         uploader = uploader_lib.TensorBoardUploader(mock_client, "/logs/foo")

From 1e69b9394b9ef9d3bca73debec7fd2448f71f006 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 12:57:50 -0500
Subject: [PATCH 05/12] adds additional testing.  Fixes a logging error

---
 tensorboard/uploader/uploader.py      |  4 +-
 tensorboard/uploader/uploader_test.py | 59 +++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/tensorboard/uploader/uploader.py b/tensorboard/uploader/uploader.py
index e3011cd72f..883ae03375 100644
--- a/tensorboard/uploader/uploader.py
+++ b/tensorboard/uploader/uploader.py
@@ -174,10 +174,10 @@ def update_experiment_metadata(
     """
     logger.info("Modifying experiment %r", experiment_id)
     if name is not None:
-        logger.info("Setting exp %r name to ", experiment_id, name)
+        logger.info("Setting exp %r name to %r", experiment_id, name)
     if description is not None:
         logger.info(
-            "Setting exp %r description to ", experiment_id, description
+            "Setting exp %r description to %r", experiment_id, description
         )
     experiment = experiment_pb2.Experiment(
         experiment_id=experiment_id, name=name, description=description
diff --git a/tensorboard/uploader/uploader_test.py b/tensorboard/uploader/uploader_test.py
index 15b34aa2e4..1cbe7bd4d3 100644
--- a/tensorboard/uploader/uploader_test.py
+++ b/tensorboard/uploader/uploader_test.py
@@ -32,6 +32,7 @@
 
 import tensorflow as tf
 
+from tensorboard.uploader.proto import experiment_pb2
 from tensorboard.uploader.proto import scalar_pb2
 from tensorboard.uploader.proto import write_service_pb2
 from tensorboard.uploader.proto import write_service_pb2_grpc
@@ -781,6 +782,64 @@ def test_internal_error(self):
         self.assertIn("travesty", msg)
 
 
+class UpdateExperimentMetadataTest(tf.test.TestCase):
+    def _create_mock_client(self):
+        # Create a stub instance (using a test channel) in order to derive a mock
+        # from it with autospec enabled. Mocking TensorBoardWriterServiceStub itself
+        # doesn't work with autospec because grpc constructs stubs via metaclassing.
+        test_channel = grpc_testing.channel(
+            service_descriptors=[], time=grpc_testing.strict_real_time()
+        )
+        stub = write_service_pb2_grpc.TensorBoardWriterServiceStub(test_channel)
+        mock_client = mock.create_autospec(stub)
+        return mock_client
+
+    def test_success(self):
+        mock_client = _create_mock_client()
+        new_name = "a new name"
+        response = write_service_pb2.UpdateExperimentResponse()
+        mock_client.UpdateExperiment.return_value = response
+
+        uploader_lib.update_experiment_metadata(
+            mock_client, "123", name=new_name)
+
+        expected_request = write_service_pb2.UpdateExperimentRequest(
+            experiment=experiment_pb2.Experiment(
+                experiment_id="123",
+                name=new_name),
+            experiment_mask=experiment_pb2.ExperimentMask(name=True)
+        )
+        mock_client.UpdateExperiment.assert_called_once()
+        (args, _) = mock_client.UpdateExperiment.call_args
+        self.assertEqual(args[0], expected_request)
+
+    def test_not_found(self):
+        mock_client = _create_mock_client()
+        error = test_util.grpc_error(grpc.StatusCode.NOT_FOUND, "nope")
+        mock_client.UpdateExperiment.side_effect = error
+
+        with self.assertRaises(uploader_lib.ExperimentNotFoundError):
+            uploader_lib.update_experiment_metadata(mock_client, "123", name="")
+
+    def test_unauthorized(self):
+        mock_client = _create_mock_client()
+        error = test_util.grpc_error(grpc.StatusCode.PERMISSION_DENIED, "nope")
+        mock_client.UpdateExperiment.side_effect = error
+
+        with self.assertRaises(uploader_lib.PermissionDeniedError):
+            uploader_lib.update_experiment_metadata(mock_client, "123", name="")
+
+    def test_internal_error(self):
+        mock_client = _create_mock_client()
+        error = test_util.grpc_error(grpc.StatusCode.INTERNAL, "travesty")
+        mock_client.UpdateExperiment.side_effect = error
+
+        with self.assertRaises(grpc.RpcError) as cm:
+            uploader_lib.update_experiment_metadata(mock_client, "123", name="")
+        msg = str(cm.exception)
+        self.assertIn("travesty", msg)
+
+
 class VarintCostTest(tf.test.TestCase):
     def test_varint_cost(self):
         self.assertEqual(uploader_lib._varint_cost(0), 1)

From a1e0ef8af0371010e8f1b48bd981b4b2f9c5dda4 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 16:14:17 -0500
Subject: [PATCH 06/12] Adds additional testing.  Changes print output to
 log.info

---
 tensorboard/uploader/uploader.py      | 27 +++++++----
 tensorboard/uploader/uploader_main.py | 18 +++----
 tensorboard/uploader/uploader_test.py | 67 +++++++++++++++++++++------
 3 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/tensorboard/uploader/uploader.py b/tensorboard/uploader/uploader.py
index 883ae03375..4225becc25 100644
--- a/tensorboard/uploader/uploader.py
+++ b/tensorboard/uploader/uploader.py
@@ -170,24 +170,22 @@ def update_experiment_metadata(
       ExperimentNotFoundError: If no such experiment exists.
       PermissionDeniedError: If the user is not authorized to modify this
         experiment.
-      RuntimeError: On unexpected failure.
+      InvalidArgumentError: If the server rejected the name or description, if,
+        for instance, the size limits have changed on the server.
     """
     logger.info("Modifying experiment %r", experiment_id)
+    request = write_service_pb2.UpdateExperimentRequest()
+    request.experiment.experiment_id = experiment_id
     if name is not None:
         logger.info("Setting exp %r name to %r", experiment_id, name)
+        request.experiment.name = name
+        request.experiment_mask.name = True
     if description is not None:
         logger.info(
             "Setting exp %r description to %r", experiment_id, description
         )
-    experiment = experiment_pb2.Experiment(
-        experiment_id=experiment_id, name=name, description=description
-    )
-    experiment_mask = experiment_pb2.ExperimentMask(
-        name=name is not None, description=description is not None
-    )
-    request = write_service_pb2.UpdateExperimentRequest(
-        experiment=experiment, experiment_mask=experiment_mask
-    )
+        request.experiment.description = description
+        request.experiment_mask.description = True
     try:
         grpc_util.call_with_retries(writer_client.UpdateExperiment, request)
     except grpc.RpcError as e:
@@ -195,6 +193,11 @@ def update_experiment_metadata(
             raise ExperimentNotFoundError()
         if e.code() == grpc.StatusCode.PERMISSION_DENIED:
             raise PermissionDeniedError()
+        if e.code() == grpc.StatusCode.INVALID_ARGUMENT:
+            details=''
+            if hasattr(e, 'details'):
+                details = e.details
+            raise InvalidArgumentError(details)
         raise
 
 
@@ -224,6 +227,10 @@ def delete_experiment(writer_client, experiment_id):
         raise
 
 
+class InvalidArgumentError(RuntimeError):
+    pass
+
+
 class ExperimentNotFoundError(RuntimeError):
     pass
 
diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index d22dde9c0d..b095198c18 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -473,11 +473,11 @@ def execute(self, server_info, channel):
             )
         except grpc.RpcError as e:
             _die("Internal error modifying experiment: %s" % e)
-        print("Modified experiment %s." % experiment_id)
+        logging.info("Modified experiment %s.", experiment_id)
         if self.name is not None:
-            print(f"Set name to {repr(self.name)}")
+            logging.info("Set name to %r", self.name)
         if self.description is not None:
-            print(f"Set description to {repr(self.description)}")
+            logging.info(f"Set description to %r", repr(self.description))
 
 
 class _ListIntent(_Intent):
@@ -527,7 +527,7 @@ def execute(self, server_info, channel):
                 ("Tags", str(experiment.num_tags)),
             ]
             for (name, value) in data:
-                print("\t%s %s" % (name.ljust(11), value))
+                print("\t%s %s" % (name.ljust(12), value))
         sys.stdout.flush()
         if not count:
             sys.stderr.write(
@@ -542,17 +542,17 @@ def _raise_if_bad_experiment_name(name):
     if name and len(name) > _EXPERIMENT_NAME_MAX_CHARS:
         raise ValueError(
             "Experiment name is too long.  Limit is "
-            f"{_EXPERIMENT_NAME_MAX_CHARS} characters.\n"
-            f"{repr(name)} was provided."
+            "%s characters.\n"`
+            "%r was provided." % (_EXPERIMENT_DESCRIPTION_MAX_CHARS, name)
         )
 
 
 def _raise_if_bad_experiment_description(description):
     if description and len(description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS:
         raise ValueError(
-            "Experiment description is too long.  Limit is "
-            f"{_EXPERIMENT_DESCRIPTION_MAX_CHARS} characters.\n"
-            f"{repr(description)} was provided."
+            "Experiment description is too long.  Limit is %s characters.\n"
+            "%r was provided." % (_EXPERIMENT_DESCRIPTION_MAX_CHARS,
+            description)
         )
 
 
diff --git a/tensorboard/uploader/uploader_test.py b/tensorboard/uploader/uploader_test.py
index 1cbe7bd4d3..618f462f82 100644
--- a/tensorboard/uploader/uploader_test.py
+++ b/tensorboard/uploader/uploader_test.py
@@ -76,24 +76,65 @@ def test_create_experiment(self):
         eid = uploader.create_experiment()
         self.assertEqual(eid, "123")
 
-    def test_create_experiment_with_name_and_description(self):
+    def test_create_experiment_with_name(self):
         logdir = "/logs/foo"
         mock_client = _create_mock_client()
-        uploader_name = uploader_lib.TensorBoardUploader(
-            mock_client, logdir, name="name"
+        new_name = "This is the new name"
+        uploader = uploader_lib.TensorBoardUploader(
+            mock_client, logdir, name=new_name
         )
-        uploader_description = uploader_lib.TensorBoardUploader(
-            mock_client, logdir, description="description"
+        eid = uploader.create_experiment()
+        self.assertEqual(eid, "123")
+        mock_client.CreateExperiment.assert_called_once()
+        (args, _) = mock_client.CreateExperiment.call_args
+
+        expected_request = write_service_pb2.CreateExperimentRequest(
+            name=new_name,
         )
-        uploader_both = uploader_lib.TensorBoardUploader(
-            mock_client, logdir, name="name", description="description"
+        self.assertEqual(args[0], expected_request)
+
+    def test_create_experiment_with_description(self):
+        logdir = "/logs/foo"
+        mock_client = _create_mock_client()
+        new_description = """
+        **description**"
+        may have "strange" unicode chars 🌴 \/<>
+        """
+        uploader = uploader_lib.TensorBoardUploader(
+            mock_client, logdir, description=new_description
+        )
+        eid = uploader.create_experiment()
+        self.assertEqual(eid, "123")
+        mock_client.CreateExperiment.assert_called_once()
+        (args, _) = mock_client.CreateExperiment.call_args
+
+        expected_request = write_service_pb2.CreateExperimentRequest(
+            description=new_description,
         )
-        eid_name = uploader_name.create_experiment()
-        self.assertEqual(eid_name, "123")
-        eid_description = uploader_description.create_experiment()
-        self.assertEqual(eid_description, "123")
-        eid_both = uploader_both.create_experiment()
-        self.assertEqual(eid_both, "123")
+        self.assertEqual(args[0], expected_request)
+
+    def test_create_experiment_with_all_metadata(self):
+        logdir = "/logs/foo"
+        mock_client = _create_mock_client()
+        new_description = """
+        **description**"
+        may have "strange" unicode chars 🌴 \/<>
+        """
+        new_name = "This is a cool name."
+        uploader = uploader_lib.TensorBoardUploader(
+            mock_client, logdir, name=new_name, description=new_description
+        )
+        eid = uploader.create_experiment()
+        self.assertEqual(eid, "123")
+        mock_client.CreateExperiment.assert_called_once()
+        (args, _) = mock_client.CreateExperiment.call_args
+
+        expected_request = write_service_pb2.CreateExperimentRequest(
+            name=new_name,
+            description=new_description,
+        )
+        self.assertEqual(args[0], expected_request)
+
 
     def test_start_uploading_without_create_experiment_fails(self):
         mock_client = _create_mock_client()

From 68b82e4ffb859dad6c511a2a1e3d6844a028428d Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 16:17:15 -0500
Subject: [PATCH 07/12] black

---
 tensorboard/uploader/uploader_test.py | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tensorboard/uploader/uploader_test.py b/tensorboard/uploader/uploader_test.py
index 618f462f82..be49d2781c 100644
--- a/tensorboard/uploader/uploader_test.py
+++ b/tensorboard/uploader/uploader_test.py
@@ -130,12 +130,10 @@ def test_create_experiment_with_all_metadata(self):
         (args, _) = mock_client.CreateExperiment.call_args
 
         expected_request = write_service_pb2.CreateExperimentRequest(
-            name=new_name,
-            description=new_description,
+            name=new_name, description=new_description,
         )
         self.assertEqual(args[0], expected_request)
 
-
     def test_start_uploading_without_create_experiment_fails(self):
         mock_client = _create_mock_client()
         uploader = uploader_lib.TensorBoardUploader(mock_client, "/logs/foo")
@@ -842,13 +840,14 @@ def test_success(self):
         mock_client.UpdateExperiment.return_value = response
 
         uploader_lib.update_experiment_metadata(
-            mock_client, "123", name=new_name)
+            mock_client, "123", name=new_name
+        )
 
         expected_request = write_service_pb2.UpdateExperimentRequest(
             experiment=experiment_pb2.Experiment(
-                experiment_id="123",
-                name=new_name),
-            experiment_mask=experiment_pb2.ExperimentMask(name=True)
+                experiment_id="123", name=new_name
+            ),
+            experiment_mask=experiment_pb2.ExperimentMask(name=True),
         )
         mock_client.UpdateExperiment.assert_called_once()
         (args, _) = mock_client.UpdateExperiment.call_args

From 41fbac39430732aa8529827dbfd75ae5cf393350 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 16:22:04 -0500
Subject: [PATCH 08/12] stray keystroke

---
 tensorboard/uploader/uploader_main.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index b095198c18..a4f3138677 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -542,7 +542,7 @@ def _raise_if_bad_experiment_name(name):
     if name and len(name) > _EXPERIMENT_NAME_MAX_CHARS:
         raise ValueError(
             "Experiment name is too long.  Limit is "
-            "%s characters.\n"`
+            "%s characters.\n"
             "%r was provided." % (_EXPERIMENT_DESCRIPTION_MAX_CHARS, name)
         )
 

From be7e7d6e4250389028dfaf3ed2e12cc4112fe995 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Fri, 21 Feb 2020 16:25:06 -0500
Subject: [PATCH 09/12] black

---
 tensorboard/uploader/uploader.py      | 4 ++--
 tensorboard/uploader/uploader_main.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tensorboard/uploader/uploader.py b/tensorboard/uploader/uploader.py
index 4225becc25..ad8f99ecd3 100644
--- a/tensorboard/uploader/uploader.py
+++ b/tensorboard/uploader/uploader.py
@@ -194,8 +194,8 @@ def update_experiment_metadata(
         if e.code() == grpc.StatusCode.PERMISSION_DENIED:
             raise PermissionDeniedError()
         if e.code() == grpc.StatusCode.INVALID_ARGUMENT:
-            details=''
-            if hasattr(e, 'details'):
+            details = ""
+            if hasattr(e, "details"):
                 details = e.details
             raise InvalidArgumentError(details)
         raise
diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index a4f3138677..805822db62 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -551,8 +551,8 @@ def _raise_if_bad_experiment_description(description):
     if description and len(description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS:
         raise ValueError(
             "Experiment description is too long.  Limit is %s characters.\n"
-            "%r was provided." % (_EXPERIMENT_DESCRIPTION_MAX_CHARS,
-            description)
+            "%r was provided."
+            % (_EXPERIMENT_DESCRIPTION_MAX_CHARS, description)
         )
 
 

From 47762aab78634434e13510294847bfabd7f4df71 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Mon, 24 Feb 2020 12:03:24 -0500
Subject: [PATCH 10/12] Adds test for INVALID_ARGUMENT.  Addresses more
 reviewer critiques

---
 tensorboard/uploader/uploader.py      |  5 +----
 tensorboard/uploader/uploader_main.py | 23 ++++++++++++++---------
 tensorboard/uploader/uploader_test.py | 14 +++++++++++++-
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/tensorboard/uploader/uploader.py b/tensorboard/uploader/uploader.py
index ad8f99ecd3..05c1344390 100644
--- a/tensorboard/uploader/uploader.py
+++ b/tensorboard/uploader/uploader.py
@@ -194,10 +194,7 @@ def update_experiment_metadata(
         if e.code() == grpc.StatusCode.PERMISSION_DENIED:
             raise PermissionDeniedError()
         if e.code() == grpc.StatusCode.INVALID_ARGUMENT:
-            details = ""
-            if hasattr(e, "details"):
-                details = e.details
-            raise InvalidArgumentError(details)
+            raise InvalidArgumentError(e.details())
         raise
 
 
diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index 805822db62..82ca0c9358 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -448,8 +448,8 @@ def execute(self, server_info, channel):
             channel
         )
         experiment_id = self.experiment_id
-        _raise_if_bad_experiment_name(self.name)
-        _raise_if_bad_experiment_description(self.description)
+        _die_if_bad_experiment_name(self.name)
+        _die_if_bad_experiment_description(self.description)
         if not experiment_id:
             raise base_plugin.FlagsError(
                 "Must specify a non-empty experiment ID to modify."
@@ -471,6 +471,11 @@ def execute(self, server_info, channel):
                 "Cannot modify experiment %s because it is owned by a "
                 "different user." % experiment_id
             )
+        except uploader_lib.InvalidArgumentError as cm:
+            _die(
+                "Server cannot modify experiment as requested.\n"
+                "Server responded: %s" % cm.description()
+            )
         except grpc.RpcError as e:
             _die("Internal error modifying experiment: %s" % e)
         logging.info("Modified experiment %s.", experiment_id)
@@ -538,18 +543,18 @@ def execute(self, server_info, channel):
         sys.stderr.flush()
 
 
-def _raise_if_bad_experiment_name(name):
+def _die_if_bad_experiment_name(name):
     if name and len(name) > _EXPERIMENT_NAME_MAX_CHARS:
-        raise ValueError(
+        _die(
             "Experiment name is too long.  Limit is "
             "%s characters.\n"
-            "%r was provided." % (_EXPERIMENT_DESCRIPTION_MAX_CHARS, name)
+            "%r was provided." % (_EXPERIMENT_NAME_MAX_CHARS, name)
         )
 
 
-def _raise_if_bad_experiment_description(description):
+def _die_if_bad_experiment_description(description):
     if description and len(description) > _EXPERIMENT_DESCRIPTION_MAX_CHARS:
-        raise ValueError(
+        _die(
             "Experiment description is too long.  Limit is %s characters.\n"
             "%r was provided."
             % (_EXPERIMENT_DESCRIPTION_MAX_CHARS, description)
@@ -583,7 +588,7 @@ def execute(self, server_info, channel):
         api_client = write_service_pb2_grpc.TensorBoardWriterServiceStub(
             channel
         )
-        _raise_if_bad_experiment_name(self.name)
+        _die_if_bad_experiment_name(self.name)
         _raise_if_bad_experiment_description(self.description)
         uploader = uploader_lib.TensorBoardUploader(
             api_client,
@@ -592,7 +597,7 @@ def execute(self, server_info, channel):
             description=self.description,
         )
         experiment_id = uploader.create_experiment()
-        url = server_info_lib.experiment_url(server_info, experiment_id)
+        udieserver_info_lib.experiment_url(server_info, experiment_id)
         print(
             "Upload started and will continue reading any new data as it's added"
         )
diff --git a/tensorboard/uploader/uploader_test.py b/tensorboard/uploader/uploader_test.py
index be49d2781c..0d9b6904b5 100644
--- a/tensorboard/uploader/uploader_test.py
+++ b/tensorboard/uploader/uploader_test.py
@@ -98,7 +98,7 @@ def test_create_experiment_with_description(self):
         mock_client = _create_mock_client()
         new_description = """
         **description**"
-        may have "strange" unicode chars 🌴 \/<>
+        may have "strange" unicode chars 🌴 \\/<>
         """
         uploader = uploader_lib.TensorBoardUploader(
             mock_client, logdir, description=new_description
@@ -869,6 +869,18 @@ def test_unauthorized(self):
         with self.assertRaises(uploader_lib.PermissionDeniedError):
             uploader_lib.update_experiment_metadata(mock_client, "123", name="")
 
+    def test_invalid_argument(self):
+        mock_client = _create_mock_client()
+        error = test_util.grpc_error(
+            grpc.StatusCode.INVALID_ARGUMENT,
+            "too many")
+        mock_client.UpdateExperiment.side_effect = error
+
+        with self.assertRaises(uploader_lib.InvalidArgumentError) as cm:
+            uploader_lib.update_experiment_metadata(mock_client, "123", name="")
+        msg = str(cm.exception)
+        self.assertIn("too many", msg)
+
     def test_internal_error(self):
         mock_client = _create_mock_client()
         error = test_util.grpc_error(grpc.StatusCode.INTERNAL, "travesty")

From 0d863a8b3491e0c39b50ed8e21c8e04962d2cdfb Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Mon, 24 Feb 2020 12:09:06 -0500
Subject: [PATCH 11/12] black & stray edit

---
 tensorboard/uploader/uploader_main.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tensorboard/uploader/uploader_main.py b/tensorboard/uploader/uploader_main.py
index 82ca0c9358..2693691dda 100644
--- a/tensorboard/uploader/uploader_main.py
+++ b/tensorboard/uploader/uploader_main.py
@@ -589,7 +589,7 @@ def execute(self, server_info, channel):
             channel
         )
         _die_if_bad_experiment_name(self.name)
-        _raise_if_bad_experiment_description(self.description)
+        _die_if_bad_experiment_description(self.description)
         uploader = uploader_lib.TensorBoardUploader(
             api_client,
             self.logdir,
@@ -597,7 +597,7 @@ def execute(self, server_info, channel):
             description=self.description,
         )
         experiment_id = uploader.create_experiment()
-        udieserver_info_lib.experiment_url(server_info, experiment_id)
+        url = server_info_lib.experiment_url(server_info, experiment_id)
         print(
             "Upload started and will continue reading any new data as it's added"
         )

From 16f46d7e743e7cc4f090d4978c55f73faa796504 Mon Sep 17 00:00:00 2001
From: Stanley Bileschi <bileschi@google.com>
Date: Mon, 24 Feb 2020 12:12:16 -0500
Subject: [PATCH 12/12] evenblacker

---
 tensorboard/uploader/uploader_test.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tensorboard/uploader/uploader_test.py b/tensorboard/uploader/uploader_test.py
index 0d9b6904b5..7f42cf9c86 100644
--- a/tensorboard/uploader/uploader_test.py
+++ b/tensorboard/uploader/uploader_test.py
@@ -872,8 +872,8 @@ def test_unauthorized(self):
     def test_invalid_argument(self):
         mock_client = _create_mock_client()
         error = test_util.grpc_error(
-            grpc.StatusCode.INVALID_ARGUMENT,
-            "too many")
+            grpc.StatusCode.INVALID_ARGUMENT, "too many"
+        )
         mock_client.UpdateExperiment.side_effect = error
 
         with self.assertRaises(uploader_lib.InvalidArgumentError) as cm: