Use tensorboard.errors in histograms and distributions (#2986)

Summary:
We now raise `errors.NotFoundError` rather than manually propagating an
error message and status code.

The response code was previously 400 Bad Request, but 404 Not Found is
more appropriate, and is consistent with the scalars plugin.

Test Plan:
Requesting `/data/plugin/histograms/histograms?run=foo&tag=bar` yields
an error with the same message as before (but now with a “Not found:”
prefix), and the histograms and distributions dashboards both work.

wchargin-branch: histograms-notfound

Author: wchargin

tensorboard/plugins/distribution/BUILD

@@ -31,6 +31,7 @@ py_test(
     deps = [
         ":compressor",
         ":distributions_plugin",
+        "//tensorboard:errors",
         "//tensorboard:expect_tensorflow_installed",
         "//tensorboard/backend:application",
         "//tensorboard/backend/event_processing:event_accumulator",

tensorboard/plugins/distribution/distributions_plugin.py

@@ -74,7 +74,11 @@ def frontend_metadata(self):
     )
 
   def distributions_impl(self, tag, run):
-    """Result of the form `(body, mime_type)`, or `ValueError`."""
+    """Result of the form `(body, mime_type)`.
+
+    Raises:
+      tensorboard.errors.PublicError: On invalid request.
+    """
     (histograms, mime_type) = self._histograms_plugin.histograms_impl(
         tag, run, downsample_to=self.SAMPLE_SIZE)
     return ([self._compress(histogram) for histogram in histograms],
@@ -98,10 +102,5 @@ def distributions_route(self, request):
     """Given a tag and single run, return an array of compressed histograms."""
     tag = request.args.get('tag')
     run = request.args.get('run')
-    try:
-      (body, mime_type) = self.distributions_impl(tag, run)
-      code = 200
-    except ValueError as e:
-      (body, mime_type) = (str(e), 'text/plain')
-      code = 400
-    return http_util.Respond(request, body, mime_type, code=code)
+    (body, mime_type) = self.distributions_impl(tag, run)
+    return http_util.Respond(request, body, mime_type)

tensorboard/plugins/distribution/distributions_plugin_test.py

@@ -22,10 +22,10 @@
 import collections
 import os.path
 
-import six
 from six.moves import xrange  # pylint: disable=redefined-builtin
 import tensorflow as tf
 
+from tensorboard import errors
 from tensorboard.backend.event_processing import plugin_event_accumulator as event_accumulator  # pylint: disable=line-too-long
 from tensorboard.backend.event_processing import plugin_event_multiplexer as event_multiplexer  # pylint: disable=line-too-long
 from tensorboard.plugins import base_plugin
@@ -137,7 +137,7 @@ def _test_distributions(self, run_name, tag_name, should_work=True):
         (bps, _unused_icdfs) = zip(*bps_and_icdfs)
         self.assertEqual(bps, compressor.NORMAL_HISTOGRAM_BPS)
     else:
-      with six.assertRaisesRegex(self, ValueError, 'No histogram tag'):
+      with self.assertRaises(errors.NotFoundError):
         self.plugin.distributions_impl(self._DISTRIBUTION_TAG, run_name)
 
   def test_distributions_with_scalars(self):

tensorboard/plugins/histogram/BUILD

@@ -17,6 +17,7 @@ py_library(
     visibility = ["//visibility:public"],
     deps = [
         ":metadata",
+        "//tensorboard:errors",
         "//tensorboard:expect_numpy_installed",
         "//tensorboard:plugin_util",
         "//tensorboard/backend:http_util",
@@ -36,6 +37,7 @@ py_test(
     deps = [
         ":histograms_plugin",
         ":summary",
+        "//tensorboard:errors",
         "//tensorboard:expect_tensorflow_installed",
         "//tensorboard/backend:application",
         "//tensorboard/backend/event_processing:event_accumulator",

tensorboard/plugins/histogram/histograms_plugin.py

@@ -29,6 +29,7 @@
 import six
 from werkzeug import wrappers
 
+from tensorboard import errors
 from tensorboard import plugin_util
 from tensorboard.backend import http_util
 from tensorboard.compat import tf
@@ -128,10 +129,13 @@ def frontend_metadata(self):
     return base_plugin.FrontendMetadata(element_name='tf-histogram-dashboard')
 
   def histograms_impl(self, tag, run, downsample_to=None):
-    """Result of the form `(body, mime_type)`, or `ValueError`.
+    """Result of the form `(body, mime_type)`.
 
     At most `downsample_to` events will be returned. If this value is
     `None`, then no downsampling will be performed.
+
+    Raises:
+      tensorboard.errors.PublicError: On invalid request.
     """
     if self._db_connection_provider:
       # Serve data from the database.
@@ -152,7 +156,9 @@ def histograms_impl(self, tag, run, downsample_to=None):
           {'run': run, 'tag': tag, 'plugin': metadata.PLUGIN_NAME})
       row = cursor.fetchone()
       if not row:
-        raise ValueError('No histogram tag %r for run %r' % (tag, run))
+        raise errors.NotFoundError(
+            'No histogram tag %r for run %r' % (tag, run)
+        )
       (tag_id,) = row
       # Fetch tensor values, optionally with linear-spaced sampling by step.
       # For steps ranging from s_min to s_max and sample size k, this query
@@ -196,7 +202,9 @@ def histograms_impl(self, tag, run, downsample_to=None):
       try:
         tensor_events = self._multiplexer.Tensors(run, tag)
       except KeyError:
-        raise ValueError('No histogram tag %r for run %r' % (tag, run))
+        raise errors.NotFoundError(
+            'No histogram tag %r for run %r' % (tag, run)
+        )
       if downsample_to is not None and len(tensor_events) > downsample_to:
         rand_indices = random.Random(0).sample(
             six.moves.xrange(len(tensor_events)), downsample_to)
@@ -228,11 +236,6 @@ def histograms_route(self, request):
     """Given a tag and single run, return array of histogram values."""
     tag = request.args.get('tag')
     run = request.args.get('run')
-    try:
-      (body, mime_type) = self.histograms_impl(
-          tag, run, downsample_to=self.SAMPLE_SIZE)
-      code = 200
-    except ValueError as e:
-      (body, mime_type) = (str(e), 'text/plain')
-      code = 400
-    return http_util.Respond(request, body, mime_type, code=code)
+    (body, mime_type) = self.histograms_impl(
+        tag, run, downsample_to=self.SAMPLE_SIZE)
+    return http_util.Respond(request, body, mime_type)

tensorboard/plugins/histogram/histograms_plugin_test.py

@@ -22,10 +22,10 @@
 import collections
 import os.path
 
-import six
 from six.moves import xrange  # pylint: disable=redefined-builtin
 import tensorflow as tf
 
+from tensorboard import errors
 from tensorboard.backend.event_processing import plugin_event_accumulator as event_accumulator  # pylint: disable=line-too-long
 from tensorboard.backend.event_processing import plugin_event_multiplexer as event_multiplexer  # pylint: disable=line-too-long
 from tensorboard.plugins import base_plugin
@@ -130,7 +130,7 @@ def _test_histograms(self, run_name, tag_name, should_work=True):
       self._check_histograms_result(tag_name, run_name, downsample=False)
       self._check_histograms_result(tag_name, run_name, downsample=True)
     else:
-      with six.assertRaisesRegex(self, ValueError, 'No histogram tag'):
+      with self.assertRaises(errors.NotFoundError):
         self.plugin.histograms_impl(self._HISTOGRAM_TAG, run_name)
 
   def _check_histograms_result(self, tag_name, run_name, downsample):