Refactor away TensorBoardWSGIApp in its current form (#2575)

Author: nfelt

tensorboard/backend/application.py

@@ -157,42 +157,14 @@ def standard_tensorboard_wsgi(flags, plugin_loaders, assets_zip_provider):
       continue
     plugins.append(plugin)
     plugin_name_to_instance[plugin.plugin_name] = plugin
-  return TensorBoardWSGIApp(flags.logdir, plugins, loading_multiplexer,
-                            reload_interval, flags.path_prefix,
-                            flags.reload_task)
 
-
-def TensorBoardWSGIApp(logdir, plugins, multiplexer, reload_interval,
-                       path_prefix='', reload_task='auto'):
-  """Constructs the TensorBoard application.
-
-  Args:
-    logdir: the logdir spec that describes where data will be loaded.
-      may be a directory, or comma,separated list of directories, or colons
-      can be used to provide named directories
-    plugins: A list of base_plugin.TBPlugin subclass instances.
-    multiplexer: The EventMultiplexer with TensorBoard data to serve
-    reload_interval: How often (in seconds) to reload the Multiplexer.
-      Zero means reload just once at startup; negative means never load.
-    path_prefix: A prefix of the path when app isn't served from root.
-    reload_task: Indicates the type of background task to reload with.
-
-  Returns:
-    A WSGI application that implements the TensorBoard backend.
-
-  Raises:
-    ValueError: If something is wrong with the plugin configuration.
-
-  :type plugins: list[base_plugin.TBPlugin]
-  :rtype: TensorBoardWSGI
-  """
-  path_to_run = parse_event_files_spec(logdir)
   if reload_interval >= 0:
     # We either reload the multiplexer once when TensorBoard starts up, or we
     # continuously reload the multiplexer.
-    start_reloading_multiplexer(multiplexer, path_to_run, reload_interval,
-                                reload_task)
-  return TensorBoardWSGI(plugins, path_prefix)
+    path_to_run = parse_event_files_spec(flags.logdir)
+    start_reloading_multiplexer(
+        loading_multiplexer, path_to_run, reload_interval, flags.reload_task)
+  return TensorBoardWSGI(plugins, flags.path_prefix)
 
 
 class TensorBoardWSGI(object):

tensorboard/backend/application_test.py

@@ -81,10 +81,10 @@ class FakePlugin(base_plugin.TBPlugin):
   """A plugin with no functionality."""
 
   def __init__(self,
-               context,
-               plugin_name,
-               is_active_value,
-               routes_mapping,
+               context=None,
+               plugin_name='foo',
+               is_active_value=True,
+               routes_mapping={},
                element_name_value=None,
                es_module_path_value=None,
                construction_callback=None):
@@ -138,19 +138,14 @@ def frontend_metadata(self):
 class ApplicationTest(tb_test.TestCase):
   def setUp(self):
     plugins = [
+        FakePlugin(plugin_name='foo'),
         FakePlugin(
-            None, plugin_name='foo', is_active_value=True, routes_mapping={}),
-        FakePlugin(
-            None,
             plugin_name='bar',
             is_active_value=False,
-            routes_mapping={},
             element_name_value='tf-bar-dashboard',
         ),
         FakePlugin(
-            None,
             plugin_name='baz',
-            is_active_value=True,
             routes_mapping={
                 '/esmodule': lambda req: None,
             },
@@ -216,19 +211,14 @@ class ApplicationBaseUrlTest(tb_test.TestCase):
   path_prefix = '/test'
   def setUp(self):
     plugins = [
+        FakePlugin(plugin_name='foo'),
         FakePlugin(
-            None, plugin_name='foo', is_active_value=True, routes_mapping={}),
-        FakePlugin(
-            None,
             plugin_name='bar',
             is_active_value=False,
-            routes_mapping={},
             element_name_value='tf-bar-dashboard',
         ),
         FakePlugin(
-            None,
             plugin_name='baz',
-            is_active_value=True,
             routes_mapping={
                 '/esmodule': lambda req: None,
             },
@@ -298,89 +288,73 @@ def testPluginsListing(self):
 
 class ApplicationPluginNameTest(tb_test.TestCase):
 
-  def _test(self, name, should_be_okay):
-    temp_dir = tempfile.mkdtemp(prefix=self.get_temp_dir())
-    self.addCleanup(shutil.rmtree, temp_dir)
-    multiplexer = event_multiplexer.EventMultiplexer(
-        size_guidance=application.DEFAULT_SIZE_GUIDANCE,
-        purge_orphaned_data=True)
-    plugins = [
-        FakePlugin(
-            None, plugin_name='foo', is_active_value=True, routes_mapping={}),
-        FakePlugin(
-            None, plugin_name=name, is_active_value=True, routes_mapping={}),
-        FakePlugin(
-            None, plugin_name='bar', is_active_value=False, routes_mapping={}),
-    ]
-    if should_be_okay:
-      application.TensorBoardWSGIApp(
-          temp_dir, plugins, multiplexer, reload_interval=0,
-          path_prefix='')
-    else:
-      with six.assertRaisesRegex(self, ValueError, r'invalid name'):
-        application.TensorBoardWSGIApp(
-            temp_dir, plugins, multiplexer, reload_interval=0,
-            path_prefix='')
+  def testSimpleName(self):
+    application.TensorBoardWSGI(
+        plugins=[FakePlugin(plugin_name='scalars')])
+
+  def testComprehensiveName(self):
+    application.TensorBoardWSGI(
+        plugins=[FakePlugin(plugin_name='Scalar-Dashboard_3000.1')])
+
+  def testNameIsNone(self):
+    with six.assertRaisesRegex(self, ValueError, r'no plugin_name'):
+      application.TensorBoardWSGI(
+          plugins=[FakePlugin(plugin_name=None)])
 
   def testEmptyName(self):
-    self._test('', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid name'):
+      application.TensorBoardWSGI(
+          plugins=[FakePlugin(plugin_name='')])
 
   def testNameWithSlashes(self):
-    self._test('scalars/data', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid name'):
+      application.TensorBoardWSGI(
+          plugins=[FakePlugin(plugin_name='scalars/data')])
 
   def testNameWithSpaces(self):
-    self._test('my favorite plugin', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid name'):
+      application.TensorBoardWSGI(
+          plugins=[FakePlugin(plugin_name='my favorite plugin')])
 
-  def testSimpleName(self):
-    self._test('scalars', True)
-
-  def testComprehensiveName(self):
-    self._test('Scalar-Dashboard_3000.1', True)
+  def testDuplicateName(self):
+    with six.assertRaisesRegex(self, ValueError, r'Duplicate'):
+      application.TensorBoardWSGI(
+          plugins=[FakePlugin(plugin_name='scalars'),
+                   FakePlugin(plugin_name='scalars')])
 
 
 class ApplicationPluginRouteTest(tb_test.TestCase):
 
-  def _test(self, route, should_be_okay):
-    temp_dir = tempfile.mkdtemp(prefix=self.get_temp_dir())
-    self.addCleanup(shutil.rmtree, temp_dir)
-    multiplexer = event_multiplexer.EventMultiplexer(
-        size_guidance=application.DEFAULT_SIZE_GUIDANCE,
-        purge_orphaned_data=True)
-    plugins = [
-        FakePlugin(
-            None,
-            plugin_name='foo',
-            is_active_value=True,
-            routes_mapping={route: lambda environ, start_response: None}),
-    ]
-    if should_be_okay:
-      application.TensorBoardWSGIApp(
-          temp_dir, plugins, multiplexer, reload_interval=0, path_prefix='')
-    else:
-      with six.assertRaisesRegex(self, ValueError, r'invalid route'):
-        application.TensorBoardWSGIApp(
-            temp_dir, plugins, multiplexer, reload_interval=0, path_prefix='')
+  def _make_plugin(self, route):
+    return FakePlugin(
+        plugin_name='foo',
+        routes_mapping={route: lambda environ, start_response: None})
 
   def testNormalRoute(self):
-    self._test('/runs', True)
+    application.TensorBoardWSGI([self._make_plugin('/runs')])
 
   def testWildcardRoute(self):
-    self._test('/foo/*', True)
+    application.TensorBoardWSGI([self._make_plugin('/foo/*')])
 
   def testNonPathComponentWildcardRoute(self):
-    self._test('/foo*', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid route'):
+      application.TensorBoardWSGI([self._make_plugin('/foo*')])
 
   def testMultiWildcardRoute(self):
-    self._test('/foo/*/bar/*', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid route'):
+      application.TensorBoardWSGI([self._make_plugin('/foo/*/bar/*')])
 
   def testInternalWildcardRoute(self):
-    self._test('/foo/*/bar', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid route'):
+      application.TensorBoardWSGI([self._make_plugin('/foo/*/bar')])
 
   def testEmptyRoute(self):
-    self._test('', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid route'):
+      application.TensorBoardWSGI([self._make_plugin('')])
 
   def testSlashlessRoute(self):
-    self._test('runaway', False)
+    with six.assertRaisesRegex(self, ValueError, r'invalid route'):
+      application.TensorBoardWSGI([self._make_plugin('runaway')])
 
 
 class GetEventFileActiveFilterTest(tb_test.TestCase):
@@ -426,9 +400,9 @@ def assertPlatformSpecificLogdirParsing(self, pathObj, logdir, expected):
         pathObj: a custom replacement object for `os.path`, typically
           `posixpath` or `ntpath`
         logdir: the string to be parsed by
-          :func:`~application.TensorBoardWSGIApp.parse_event_files_spec`
+          :func:`~application.parse_event_files_spec`
         expected: the expected dictionary as returned by
-          :func:`~application.TensorBoardWSGIApp.parse_event_files_spec`
+          :func:`~application.parse_event_files_spec`
 
     """
 
@@ -662,32 +636,6 @@ def testEmptyWildcardRouteWithSlash(self):
     self._test_route('/data/plugin/bar/wildcard/', 404)
 
 
-class ApplicationConstructionTest(tb_test.TestCase):
-
-  def testExceptions(self):
-    logdir = '/fake/foo'
-    multiplexer = event_multiplexer.EventMultiplexer()
-
-    # Fails if there is an unnamed plugin
-    with self.assertRaises(ValueError):
-      # This plugin lacks a name.
-      plugins = [
-          FakePlugin(
-              None, plugin_name=None, is_active_value=True, routes_mapping={}),
-      ]
-      application.TensorBoardWSGIApp(logdir, plugins, multiplexer, 0, '')
-
-    # Fails if there are two plugins with same name
-    with self.assertRaises(ValueError):
-      plugins = [
-          FakePlugin(
-              None, plugin_name='foo', is_active_value=True, routes_mapping={}),
-          FakePlugin(
-              None, plugin_name='foo', is_active_value=True, routes_mapping={}),
-      ]
-      application.TensorBoardWSGIApp(logdir, plugins, multiplexer, 0, '')
-
-
 class DbTest(tb_test.TestCase):
 
   def testSqliteDb(self):

tensorboard/plugins/audio/audio_plugin_test.py

@@ -92,18 +92,12 @@ def setUp(self):
         "foo": foo_directory,
         "bar": bar_directory,
     })
+    multiplexer.Reload()
     context = base_plugin.TBContext(
         logdir=self.log_dir, multiplexer=multiplexer)
     self.plugin = audio_plugin.AudioPlugin(context)
-    # Setting a reload interval of -1 disables reloading. We disable reloading
-    # because we seek to block tests from running til after one reload finishes.
-    # This setUp method thus manually reloads the multiplexer. TensorBoard would
-    # otherwise reload in a non-blocking thread.
-    wsgi_app = application.TensorBoardWSGIApp(
-        self.log_dir, [self.plugin], multiplexer, reload_interval=-1,
-        path_prefix='')
+    wsgi_app = application.TensorBoardWSGI([self.plugin])
     self.server = werkzeug_test.Client(wsgi_app, wrappers.BaseResponse)
-    multiplexer.Reload()
 
   def tearDown(self):
     shutil.rmtree(self.log_dir, ignore_errors=True)

tensorboard/plugins/core/core_plugin_test.py

@@ -302,12 +302,7 @@ def _start_logdir_based_server(self, temp_dir):
         multiplexer=self.multiplexer,
         window_title='title foo')
     self.logdir_based_plugin = core_plugin.CorePlugin(context)
-    app = application.TensorBoardWSGIApp(
-        self.logdir,
-        [self.logdir_based_plugin],
-        self.multiplexer,
-        0,
-        path_prefix='')
+    app = application.TensorBoardWSGI([self.logdir_based_plugin])
     self.logdir_based_server = werkzeug_test.Client(app, wrappers.BaseResponse)
 
   def _start_db_based_server(self):

tensorboard/plugins/debugger/debugger_plugin_testlib.py

@@ -120,10 +120,11 @@ def setUp(self):
     writer.Close()
 
     # Start a server that will receive requests and respond with health pills.
-    self.multiplexer = event_multiplexer.EventMultiplexer({
+    multiplexer = event_multiplexer.EventMultiplexer({
         '.': self.log_dir,
         'run_foo': run_foo_directory,
     })
+    multiplexer.Reload()
     self.debugger_data_server_grpc_port = portpicker.pick_unused_port()
 
     # Fake threading behavior so that threads are synchronous.
@@ -141,12 +142,10 @@ def setUp(self):
         self.mock_debugger_data_server_class).start()
 
     self.context = base_plugin.TBContext(
-        logdir=self.log_dir, multiplexer=self.multiplexer)
+        logdir=self.log_dir, multiplexer=multiplexer)
     self.plugin = debugger_plugin.DebuggerPlugin(self.context)
     self.plugin.listen(self.debugger_data_server_grpc_port)
-    wsgi_app = application.TensorBoardWSGIApp(
-        self.log_dir, [self.plugin], self.multiplexer, reload_interval=0,
-        path_prefix='')
+    wsgi_app = application.TensorBoardWSGI([self.plugin])
     self.server = werkzeug_test.Client(wsgi_app, wrappers.BaseResponse)
 
     # The debugger data server should be started at the correct port.

tensorboard/plugins/debugger/interactive_debugger_plugin_test.py

@@ -55,21 +55,16 @@ def setUp(self):
     super(InteractiveDebuggerPluginTest, self).setUp()
 
     self._dummy_logdir = tempfile.mkdtemp()
-    self._dummy_multiplexer = event_multiplexer.EventMultiplexer({})
+    dummy_multiplexer = event_multiplexer.EventMultiplexer({})
     self._debugger_port = portpicker.pick_unused_port()
     self._debugger_url = 'grpc://localhost:%d' % self._debugger_port
     context = base_plugin.TBContext(logdir=self._dummy_logdir,
-                                    multiplexer=self._dummy_multiplexer)
+                                    multiplexer=dummy_multiplexer)
     self._debugger_plugin = (
         interactive_debugger_plugin.InteractiveDebuggerPlugin(context))
     self._debugger_plugin.listen(self._debugger_port)
 
-    wsgi_app = application.TensorBoardWSGIApp(
-        self._dummy_logdir,
-        [self._debugger_plugin],
-        self._dummy_multiplexer,
-        reload_interval=0,
-        path_prefix='')
+    wsgi_app = application.TensorBoardWSGI([self._debugger_plugin])
     self._server = werkzeug_test.Client(wsgi_app, wrappers.BaseResponse)
 
   def tearDown(self):

tensorboard/plugins/image/images_plugin_test.py

@@ -86,17 +86,12 @@ def setUp(self):
         "foo": foo_directory,
         "bar": bar_directory,
     })
+    multiplexer.Reload()
     context = base_plugin.TBContext(
         logdir=self.log_dir, multiplexer=multiplexer)
     plugin = images_plugin.ImagesPlugin(context)
-    # Setting a reload interval of -1 disables reloading. We disable reloading
-    # because we seek to block tests from running til after one reload finishes.
-    # This setUp method thus manually reloads the multiplexer. TensorBoard would
-    # otherwise reload in a non-blocking thread.
-    wsgi_app = application.TensorBoardWSGIApp(
-        self.log_dir, [plugin], multiplexer, reload_interval=-1, path_prefix='')
+    wsgi_app = application.TensorBoardWSGI([plugin])
     self.server = werkzeug_test.Client(wsgi_app, wrappers.BaseResponse)
-    multiplexer.Reload()
     self.routes = plugin.get_plugin_apps()
 
   def tearDown(self):