Skip to content

Serve TensorBoard at /flags #5796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rileyajones wants to merge 1 commit into from
Closed

Serve TensorBoard at /flags #5796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 31 additions & 8 deletions tensorboard/plugins/core/core_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
# transition to 'text/javascript'.
JS_MIMETYPES = ["text/javascript", "application/javascript"]
JS_CACHE_EXPIRATION_IN_SECS = 86400
# Th paths that the application should be served at
# Note that paths other than "/" should NOT end in '/' because trailing '/'
# characters are removed by the serving logic in tensorboard/backend/application.py
APP_PATHS = ["/", "/flags"]


class CorePlugin(base_plugin.TBPlugin):
Expand Down Expand Up @@ -109,17 +113,34 @@ def get_resource_apps(self):
content = zip_.read(path)
# Opt out of gzipping index.html
if path == "index.html":
apps["/" + path] = functools.partial(
self._serve_index, content
)
for app_path in APP_PATHS:
path_with_slash = (
app_path
if app_path.endswith("/")
else app_path + "/"
)
apps[path_with_slash + path] = functools.partial(
self._serve_index, content, app_path
)
continue

gzipped_asset_bytes = _gzip(content)
wsgi_app = functools.partial(
self._serve_asset, path, gzipped_asset_bytes
)
apps["/" + path] = wsgi_app
apps["/"] = apps["/index.html"]
for app_path in APP_PATHS:
path_with_slash = (
app_path
if app_path.endswith("/")
else app_path + "/"
)
apps[path_with_slash + path] = wsgi_app
for app_path in APP_PATHS:
if app_path.endswith("/"):
apps[app_path] = apps[app_path + "index.html"]
else:
apps[app_path] = apps[app_path + "/index.html"]
apps[app_path] = apps[app_path]
Comment on lines +116 to +143
Copy link
Contributor Author

@rileyajones rileyajones Jul 8, 2022

Choose a reason for hiding this comment

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

This DEFINITELY needs a unit test

Copy link
Contributor

@JamesHollyer JamesHollyer Jul 11, 2022

Choose a reason for hiding this comment

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

Unit test here would be great. I don't understand what line 143 does.

return apps

@wrappers.Request.application
Expand Down Expand Up @@ -151,21 +172,23 @@ def _serve_asset(self, path, gzipped_asset_bytes, request):
)

@wrappers.Request.application
def _serve_index(self, index_asset_bytes, request):
def _serve_index(self, index_asset_bytes, content_path, request):
"""Serves index.html content.

Note that we opt out of gzipping index.html to write preamble before the
resource content. This inflates the resource size from 2x kiB to 1xx
kiB, but we require an ability to flush preamble with the HTML content.
"""
if not request.path.endswith("/"):
return utils.redirect(request.path + "/")
relpath = (
posixpath.relpath(self._path_prefix, request.script_root)
if self._path_prefix
else "."
)
meta_header = (
'<!doctype html><meta name="tb-relative-root" content="%s/">'
% relpath
'<!doctype html><meta name="tb-relative-root" content="%s%s">'
% (relpath, content_path)
)
content = meta_header.encode("utf-8") + index_asset_bytes
# By passing content_encoding, disallow gzipping. Bloats the content
Expand Down
9 changes: 8 additions & 1 deletion tensorboard/webapp/app_routing/programmatical_navigation_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,17 @@ export interface NavigateToExperiments {
resetNamespacedState?: boolean;
}

export interface NavigateToFlags {
routeKind: RouteKind.FLAGS;
routeParams: {},
resetNamespacedState?: boolean;
}

export type ProgrammaticalNavigation =
| NavigateToExperiment
| NavigateToCompare
| NavigateToExperiments;
| NavigateToExperiments
| NavigateToFlags;
Comment on lines +43 to +53
Copy link
Contributor Author

@rileyajones rileyajones Jul 7, 2022

Choose a reason for hiding this comment

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

This is not technically necessary but it does make the typing more accurate.

Copy link
Contributor

@JamesHollyer JamesHollyer Jul 11, 2022

Choose a reason for hiding this comment

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

I don't have strong feelings about this either way. However, I would have left this out because we do not want to programmatically navigate.

I can definitely see this being useful one day if we decide to have a secret button that navigates here or something. But, I think we can add it then.


export interface NavigationLambda {
actionCreator: ActionCreator<string, Creator>;
Expand Down