Experimental plugins

[bmd3k]

• Motivation for features / changes

Allow an administrator of a Tensorboard to mark certain plugins as "experimental" and hidden to users by default. Users would then need to add query parameter "experimentalPlugin=" to the URL of their experiment to see the experimental plugin.

• Technical description of changes

standard_tensorboard_wsgi and TensorBoardWSGIApp include an argument "experimental_plugins" that lists plugins, by name, that are in an experimental state.

There is logic in _serve_plugins_listing() that filters out plugins that are included in experimental_plugins but are not specified as a value for an experimentalPlugin query parameter.

There is logic in createRouter() that forward "experimentalPlugin" query parameter values from the original TensorBoard request to the call to the plugins_listing endpoint.

• Screenshots of UI changes

Using an example where "images" and "graphs" are marked as experimental.

No query parameters:

"experimentalPlugin=images" query parameter:

"experimentalPlugin=images&experimentalPlugin=graphs" query parameters:

• Detailed steps to verify changes work correctly (as executed by you)

Modify your favorite instance of Tensorboard to include experimental_plugins = ["images", "graphs"] in call to standard_tensorboard_wsgi() or TensorBoardWSGIApp().

Start server.

Open "http://localhost:6006/" without query parameters. Note that "Images" and "Graphs" do not appear as plugins.

Open "http://localhost:6006/?experimentalPlugin=images&experimentalPlugin=graphs". Note that "Images" and "Graphs" appear as plugins.

[wchargin]

Comments modulo agreement about whether the args to TensorBoardWSGIApp
are okay with us (cc @nfelt).

[wchargin]

Use experimental_plugins=None instead and do the resolution inside the
function body, because:

• default argument values are evaluated at function definition time,
  not invocation time, so this introduces a globally shared mutable
  list object—not what you want! (cf. style guide)
• using None as a default argument value in general is a common
  Python pattern enabling callers to specify “use default”
  without having to duplicate the actual value of the default

(Likewise below.)

[bmd3k]

Done.
Wow, mutable shared default values.
I'm not sure if the way I now initialize self._experimental_plugins is idiomatic python - let me know!

[wchargin]

Could we call this experimentalPlugin instead? exp is ambiguous in
general, and especially so given that TensorBoard has a notion of
experiments that’s not what’s meant here.

[bmd3k]

Done.
Yes, I think it's fine to be a bit wordy for now assuming this isn't used very frequently. If this proves to be a popular feature, though, we may find ourselves adding something very concise later on. Previous systems I've worked with use "e=" to trigger experiments. We might want to add something like "ep" in the future. Maybe.

[wchargin]

Since this is only used for membership tests, best to convert it to a
frozenset for faster queries and more idiomatic set operations (see
suggestion below).

[bmd3k]

Done.

[wchargin]

Instead of filter(lambda: ..., ...), use a list comprehension, per the
style guide:
https://google.github.io/styleguide/pyguide.html#215-deprecated-language-features

But probably simpler here to just conditionally continue at the top of
the for-loop? Certainly shorter:

        plugins_to_skip = self._experimental_plugins - frozenset(
            request.args.getlist(EXPERIMENTAL_PLUGINS_QUERY_PARAM)
        )
        for plugin in self._plugins:
            if p.plugin_name in plugins_to_skip:
                continue
            ...

[bmd3k]

Done. (2nd option)

[wchargin]

Nice tests; thanks!

[wchargin]

This is a little scary; in the past, we’ve had issues with code like
this breaking --path_prefix handling. It’s fine here because you’re
only using it for the search params rather than the path, but I wonder
if we can make that clearer—something like

  export function createRouter(
    dataDir = 'data',
    searchParams = new URLSearchParams(window.location.search)
  ): Router {
    // ...
  }

such that future changes to the function don’t accidentally rely on
path?

[bmd3k]

Done. Though I decided to call it urlSearchParams.

[wchargin]

Hmm; it would be great if we could avoid exposing this on program;
unlike backend.application, program is a public API and I’m not sure
that we want to commit to supporting this forever.

Do you have any thoughts about the best way to do this? The internal
clients that need this don’t call through program (instead calling
TensorBoardWSGIApp directly), but I see that tensorboard.main does.

[bmd3k]

It's true that for my particular use case I do not need to call through program.py. I made the changes here just to be complete. I'm ok reverting the changes to this file and deciding later whether to expose the experimentalPlugin functionality more broadly.

[wchargin]

(For posterity, this is superseded by #3344.)