Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions tensorboard/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ package_group(
py_binary(
name = "tensorboard",
srcs = ["main.py"],
data = ["webfiles.zip"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I agree this is where the data dep belongs, but right now the code in main.py actually reads it from program.get_default_assets_zip_provider(). It used to live in default but was moved to program back in #1631 (not really sure why, at this point).

IMO better at this point to just inline it into main.py and remove the fallback logic in program.TensorBoard() that calls it, so that assets_zip_provider must be specified by the caller. That way we don't have skew between where the data dep is defined in the BUILD file, and where the code actually expects it to exist. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Done.

main = "main.py",
srcs_version = "PY3",
deps = [
":default",
":dynamic_plugins",
":dynamic_plugins", # loads internal dynamic plugin like projector
":lib",
":main_lib",
":program",
Expand All @@ -45,6 +46,22 @@ py_library(
],
)

py_binary(
name = "dev",
srcs = ["main_dev.py"],
data = ["dev_webfiles.zip"],
main = "main_dev.py",
srcs_version = "PY3",
deps = [
":default",
":dynamic_plugins", # loads internal dynamic plugin like projector
":lib",
":main_lib",
":program",
"//tensorboard/uploader:uploader_subcommand",
],
)

# The public TensorBoard python library, bundled with the pip package and
# available via 'import tensorboard as tb' once installed.
py_library(
Expand Down Expand Up @@ -248,7 +265,6 @@ py_library(
py_library(
name = "default",
srcs = ["default.py"],
data = ["webfiles.zip"],
srcs_version = "PY3",
deps = [
"//tensorboard:expect_pkg_resources_installed",
Expand Down Expand Up @@ -336,6 +352,29 @@ tf_web_library(
],
)

tensorboard_zip_file(
name = "dev_webfiles",
deps = [":dev_assets"],
)

# Should keep the asset dependencies in sync with :assets.
tf_web_library(
name = "dev_assets",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the idea is that we'll replace this fairly soon with something more substantially different? Otherwise, would be good to have a comment saying to keep the srcs/deps in sync with the non-dev assets.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, #4589 gives you glimpse of what the difference might be.

srcs = [
"//tensorboard/webapp:index.html",
"//tensorboard/webapp:index.js",
"//tensorboard/webapp:svg_bundle",
"//tensorboard/webapp/widgets/line_chart_v2/lib/worker:chart_worker.js",
],
path = "/",
suppress = ["strictDependencies"],
deps = [
"//tensorboard/webapp/widgets/source_code/monaco:monaco_editor",
"//tensorboard/webapp/widgets/source_code/monaco:monaco_languages",
"@com_google_fonts_roboto",
],
)

# This is a dummy rule used as a numpy dependency in open-source.
# We expect numpy to already be installed on the system, e.g. via
# `pip install numpy`
Expand Down
19 changes: 16 additions & 3 deletions tensorboard/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
TensorBoard uses can swap out this file with their own.
"""


import inspect
import os
import sys

from absl import app
Expand All @@ -30,14 +31,26 @@
from tensorboard import program
from tensorboard.plugins import base_plugin
from tensorboard.uploader import uploader_subcommand
from tensorboard.util import tb_logging

logger = tb_logging.get_logger()


def run_main():
"""Initializes flags and calls main()."""
main_lib.global_init()

path = os.path.join(
os.path.dirname(inspect.getfile(sys._getframe(0))), "webfiles.zip"
)

if not os.path.exists(path):
logger.warning("webfiles.zip static assets not found: %s", path)
return None

tensorboard = program.TensorBoard(
default.get_plugins(),
program.get_default_assets_zip_provider(),
lambda: open(path, "rb"),
plugins=default.get_plugins(),
subcommands=[uploader_subcommand.UploaderSubcommand()],
)
try:
Expand Down
51 changes: 51 additions & 0 deletions tensorboard/main_dev.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Copyright 2021 The TensorFlow Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# ==============================================================================
"""TensorBoard dev main module."""

import inspect
import os
import sys

from absl import app
from tensorboard import default
from tensorboard import main_lib
from tensorboard import program
from tensorboard.plugins import base_plugin
from tensorboard.uploader import uploader_subcommand


def run_main():
"""Initializes flags and calls main()."""
main_lib.global_init()

path = os.path.join(
os.path.dirname(inspect.getfile(sys._getframe(0))), "dev_webfiles.zip"
)

tensorboard = program.TensorBoard(
lambda: open(path, "rb"),
plugins=default.get_plugins(),
subcommands=[uploader_subcommand.UploaderSubcommand()],
)

try:
app.run(tensorboard.main, flags_parser=tensorboard.configure)
except base_plugin.FlagsError as e:
print("Error: %s" % e, file=sys.stderr)
sys.exit(1)


if __name__ == "__main__":
run_main()
26 changes: 3 additions & 23 deletions tensorboard/program.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import atexit
from collections import defaultdict
import errno
import inspect
import logging
import mimetypes
import os
Expand Down Expand Up @@ -65,24 +64,6 @@
_SUBCOMMAND_FLAG = "__tensorboard_subcommand"


def get_default_assets_zip_provider():
"""Opens stock TensorBoard web assets collection.

Returns:
Returns function that returns a newly opened file handle to zip file
containing static assets for stock TensorBoard, or None if webfiles.zip
could not be found. The value the callback returns must be closed. The
paths inside the zip file are considered absolute paths on the web server.
"""
path = os.path.join(
os.path.dirname(inspect.getfile(sys._getframe(1))), "webfiles.zip"
)
if not os.path.exists(path):
logger.warning("webfiles.zip static assets not found: %s", path)
return None
return lambda: open(path, "rb")


class TensorBoard(object):
"""Class for running TensorBoard.

Expand All @@ -96,18 +77,19 @@ class TensorBoard(object):

def __init__(
self,
assets_zip_provider,
plugins=None,
assets_zip_provider=None,
server_class=None,
subcommands=None,
):
"""Creates new instance.

Args:
assets_zip_provider: A function that provides a zip file containing assets to
the application.
plugins: A list of TensorBoard plugins to load, as TBPlugin classes or
TBLoader instances or classes. If not specified, defaults to first-party
plugins.
assets_zip_provider: Delegates to TBContext or uses default if None.
server_class: An optional factory for a `TensorBoardServer` to use
for serving the TensorBoard WSGI app. If provided, its callable
signature should match that of `TensorBoardServer.__init__`.
Expand All @@ -122,8 +104,6 @@ def __init__(
from tensorboard import default

plugins = default.get_plugins()
if assets_zip_provider is None:
assets_zip_provider = get_default_assets_zip_provider()
if server_class is None:
server_class = create_port_scanning_werkzeug_server
if subcommands is None:
Expand Down
34 changes: 28 additions & 6 deletions tensorboard/program_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,37 +26,51 @@
from tensorboard.plugins.core import core_plugin


def fake_asset_provider():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wanted to avoid the boilerplate here we could always make the argument still optional, but if omitted, we would generate a dummy zip file that just contains index.html with Hello TensorBoard or something. Probably fine to make it required, but just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer things to be a bit more explicit so I will keep it as is.

pass


class TensorBoardTest(tb_test.TestCase):
"""Tests the TensorBoard program."""

def testPlugins_pluginClass(self):
tb = program.TensorBoard(plugins=[core_plugin.CorePlugin])
tb = program.TensorBoard(
fake_asset_provider, plugins=[core_plugin.CorePlugin]
)
self.assertIsInstance(tb.plugin_loaders[0], base_plugin.BasicLoader)
self.assertIs(tb.plugin_loaders[0].plugin_class, core_plugin.CorePlugin)

def testPlugins_pluginLoaderClass(self):
tb = program.TensorBoard(plugins=[core_plugin.CorePluginLoader])
tb = program.TensorBoard(
fake_asset_provider, plugins=[core_plugin.CorePluginLoader]
)
self.assertIsInstance(
tb.plugin_loaders[0], core_plugin.CorePluginLoader
)

def testPlugins_pluginLoader(self):
loader = core_plugin.CorePluginLoader()
tb = program.TensorBoard(plugins=[loader])
tb = program.TensorBoard(fake_asset_provider, plugins=[loader])
self.assertIs(tb.plugin_loaders[0], loader)

def testPlugins_invalidType(self):
plugin_instance = core_plugin.CorePlugin(base_plugin.TBContext())
with self.assertRaisesRegex(TypeError, "CorePlugin"):
tb = program.TensorBoard(plugins=[plugin_instance])
tb = program.TensorBoard(
fake_asset_provider, plugins=[plugin_instance]
)

def testConfigure(self):
tb = program.TensorBoard(plugins=[core_plugin.CorePluginLoader])
tb = program.TensorBoard(
fake_asset_provider, plugins=[core_plugin.CorePluginLoader]
)
tb.configure(logdir="foo")
self.assertEqual(tb.flags.logdir, "foo")

def testConfigure_unknownFlag(self):
tb = program.TensorBoard(plugins=[core_plugin.CorePlugin])
tb = program.TensorBoard(
fake_asset_provider, plugins=[core_plugin.CorePlugin]
)
with self.assertRaisesRegex(ValueError, "Unknown TensorBoard flag"):
tb.configure(foo="bar")

Expand Down Expand Up @@ -150,6 +164,7 @@ def tearDown(self):

def testImplicitServe(self):
tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand(lambda parser: None)],
)
Expand All @@ -162,6 +177,7 @@ def testImplicitServe(self):

def testExplicitServe(self):
tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand()],
)
Expand All @@ -179,6 +195,7 @@ def define_flags(parser):
parser.add_argument("--hello")

tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand(define_flags=define_flags)],
)
Expand All @@ -190,6 +207,7 @@ def define_flags(parser):

def testSubcommand_ExitCode(self):
tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand()],
)
Expand All @@ -199,6 +217,7 @@ def testSubcommand_ExitCode(self):

def testSubcommand_DoesNotInheritBaseArgs(self):
tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand()],
)
Expand All @@ -214,6 +233,7 @@ def define_flags(parser):
parser.add_argument("payload")

tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand(define_flags=define_flags)],
)
Expand All @@ -226,6 +246,7 @@ def define_flags(parser):
def testConflictingNames_AmongSubcommands(self):
with self.assertRaises(ValueError) as cm:
tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand(), _TestSubcommand()],
)
Expand All @@ -235,6 +256,7 @@ def testConflictingNames_AmongSubcommands(self):
def testConflictingNames_WithServe(self):
with self.assertRaises(ValueError) as cm:
tb = program.TensorBoard(
fake_asset_provider,
plugins=[core_plugin.CorePluginLoader],
subcommands=[_TestSubcommand(name="serve")],
)
Expand Down