RFC: TensorBoard - Improved Plugin Extensibility #90
Conversation
stephanwlee
requested review from
ewilderj,
goldiegadde and
martinwicke
as code owners
| ### Plugin distribution | ||
| A plugin will be distributed as a single Pip distribution package that includes both a Python backend and a web frontend. | ||
|
|
||
| The Python backend must export an object conforming to a standard interface that describes the plugin’s metadata and provides an option to load the plugin’s implementation. The exact shape is not yet specified, but it will be conceptually similar to a function returning a tuple containing a name, a description, and a [`tensorboard.plugins.base_plugin.TBLoader`][tb-loader] object. |
There was a problem hiding this comment.
... an object conforming to ...
Should it be an object or a function? The get_plugin example below is a function.
There was a problem hiding this comment.
Functions are objects in Python.* We use the term “object” here
generically (as Python does).
* isinstance(lambda: None, object) # True
|
|
||
| One critical drawback of all non-iframe approaches is that TensorBoard cannot enforce plugin authors from inadvertently mutating the global object. For instance, if a library that a plugin transitively depends polyfills and monkey-patches a global object, it can influence how other plugins behave. | ||
|
|
||
| **Pro** |
There was a problem hiding this comment.
See my comment above. The structure of the text around this point is somewhat hard to follow. It's unclear
- what the Pro and Con sections refer to
- why these sections exist for some of the options but not others
There was a problem hiding this comment.
+1. What's confusing here is that the options 1, 2, and 3 are at different levels of the hierarchy. IIUC, the intent is:
- Option A: Non-iFrame
- Pro, con
- Option A1
- Option A2
- Option B: iFrame
- Pro, con
|
|
||
| The Python backend must export an object conforming to a standard interface that describes the plugin’s metadata and provides an option to load the plugin’s implementation. The exact shape is not yet specified, but it will be conceptually similar to a function returning a tuple containing a name, a description, and a [`tensorboard.plugins.base_plugin.TBLoader`][tb-loader] object. | ||
|
|
||
| In order to be discovered by TensorBoard, the distribution should include an [`entry_points` entry][entry-points] declaring itself as a TensorBoard plugin. The entry point group is `tensorboard_plugins`. The entry point name is arbitrary, and ignored. The entry point object reference should refer to the standard interface described in the previous paragraph. As an example, the setup.py file for a hypothetical “widgets” plugin might look like: |
There was a problem hiding this comment.
standard interface describe in the previous paragraph
I can't find a detailed description of any interface in the preceding paragraph. Am I missing something? I see you said "The exact shape is not yet specified." But can you at least come up with an outline? There is already such a plugin registration method in the existing TensorBoard plugin system. Is the new interface expected to be similar or significantly different. I feel this RfC should at least include some rudimentary information on this topic.
There was a problem hiding this comment.
Correct, the shape is not yet specified; the outline is, “conceptually
similar to a function returning a tuple containing a name, a
description, and a loader object”. Maybe it’ll just end up being a
function that returns a plugin loader, if we determine that we don’t
need any extra metadata.
The existing plugin system requires you to write your own main module
to register plugins, whereas this new system will not.
We haven’t given a ton of thought to the details of this interface,
because we expect it to be simple for anyone to implement. We’ll pick
something and specify it clearly, and people can just use it.
|
|
||
| Instead, TensorBoard can provide a library one of which will be a `request`. The method will use `window.postMessage` to route the network request to the main frame and hide away the fact that it is using inter-frame communication at all. This solution, though easy for XMLHttpRequest based request, is difficult to apply for, for example, image, audio, video, CSS, web-fonts, EsModule, and WebSocket requests. To illustrate the problem more concretely, when `<img src="foo.png">` is created, a browser initiates the requests and it is not possible to route that request via the library. This will make a plugin like Images plugin difficult to write. | ||
|
|
||
| ## Questions and Discussion Topics |
There was a problem hiding this comment.
Big missing pieces I see in this draft
- What is the signature of
get_plugin(), i.e., how tensorboard discovers the 3p plugin? See my comment about the "standard interface" above. - Apart from the standard interface, are there going to be any new protocols for any of the following?
- data storage in the backend (e.g., tensorboard core can provide a unified database write/read API)
- communication protocol between the plugin's frontend component and the backends (e.g., tensorboard can provide an NPM package that helps the frontend component manage the requests to the backend).
There was a problem hiding this comment.
+1
Building on the discussion of the communication protocol, Does this illustration capture how you expect the pattern of communication to work? Or do you rather expect the TensorBoard FE to pass an address to the Plugin FE so that the Plugin FE can talk to the backend components directly?
There was a problem hiding this comment.
@bileschi thanks for the feedback. At this moment, we would like to confine the scope of design to redo the distribution strategy. The architecture you are describing has more profound impact than depicted in the picture and should be redesigned holistically (including the summary system and storage system. Note that the storage should also be pluggable: data format, storage mechanism, and etc...).
Before closing it, I would also like to note that close coupling between frontend and backend (and distributed as one unit) alleviates the concern of fragmentation raised below. Though hard to avoid duplication of features and varying level of quality, current plugin system allow us to do without API versioning and conflict resolutions.
There was a problem hiding this comment.
Thanks for the feedback! Can you provide a substitute diagram which describes the proposed communication strategy?
There was a problem hiding this comment.
I also strongly believe that we should reconsider the architecture of providing plugins as coupled (Backend, Frontend) pairs.
At the same time, I think that problem is somewhat separable from the current proposal. That is, this RFC goes one step in the right direction; we'd like to discuss additional related steps soon, but they don't need to block this one.
In particular I think we can easily get consensus on these bits:
- Plugins providing BE Python code are dynamically loaded at runtime.
- BE plugins are distributed via PIP.
While these aspects of the current architecture (which this RFC retains) should remain open topics:
- Plugins bundle FE code.
- The TensorBoard backend is responsible for assembling the FE app from the available plugins and serving it.
- Plugin network APIs are not formalized or versioned.
The reason this is coming up now is that most of this doc is specifically about how the plugin FEs should be assembled into the FE app that TB serves. Under a decoupled architecture in which the FE is built and served as an independent entity (presumably, from components distributed via npm, bundled with webpack or similar, etc.), the discussion around whether or not to use iFrames etc. would be very different.
The upshot is I think it's OK to go ahead with this proposal, because the dynamic BE plugin loading (and PIP distribution of those) is great in any case. It may turn out that the FE-assembly questions here will be obviated by a later design of broader scope; let's give that the attention it deserves in a separate RFC.
There was a problem hiding this comment.
@bileschi As I understand it, the current proposal does not change the existing communication strategy, which is that a plugin FE makes requests only to routes that its matched BE exposes (or to standard routes that the TB core provides). See https://github.com/tensorflow/tensorboard/blob/master/tensorboard/http_api.md. There is no client-side TB code mediating those requests, and no client-side state shared between FE plugins. (Those are certainly good things to discuss; they're just outside the present scope.)
|
There were some general comments related to plugins/modularities in the Tensorflow addons RFC starting from #84 (comment). |
|
|
||
| **Con** | ||
| - global mutating libraries like Polymer app will not work | ||
| - can lead to an awkward UX: e.g., different version of a visualization library have different UI/UX |
There was a problem hiding this comment.
I'm not sure I follow-- can you explain this concern further or give an example?
| - does not work in Colab | ||
| - generally larger binaries | ||
| - Cannot reuse components across plugins | ||
| - can lead to an awkward UX: e.g., different version of a visualization library have different UI/UX |
There was a problem hiding this comment.
This a Con of both options? Is there any alternative that avoids it, then?
|
|
||
| Instead, TensorBoard can provide a library one of which will be a `request`. The method will use `window.postMessage` to route the network request to the main frame and hide away the fact that it is using inter-frame communication at all. This solution, though easy for XMLHttpRequest based request, is difficult to apply for, for example, image, audio, video, CSS, web-fonts, EsModule, and WebSocket requests. To illustrate the problem more concretely, when `<img src="foo.png">` is created, a browser initiates the requests and it is not possible to route that request via the library. This will make a plugin like Images plugin difficult to write. | ||
|
|
||
| ## Questions and Discussion Topics |
There was a problem hiding this comment.
I also strongly believe that we should reconsider the architecture of providing plugins as coupled (Backend, Frontend) pairs.
At the same time, I think that problem is somewhat separable from the current proposal. That is, this RFC goes one step in the right direction; we'd like to discuss additional related steps soon, but they don't need to block this one.
In particular I think we can easily get consensus on these bits:
- Plugins providing BE Python code are dynamically loaded at runtime.
- BE plugins are distributed via PIP.
While these aspects of the current architecture (which this RFC retains) should remain open topics:
- Plugins bundle FE code.
- The TensorBoard backend is responsible for assembling the FE app from the available plugins and serving it.
- Plugin network APIs are not formalized or versioned.
The reason this is coming up now is that most of this doc is specifically about how the plugin FEs should be assembled into the FE app that TB serves. Under a decoupled architecture in which the FE is built and served as an independent entity (presumably, from components distributed via npm, bundled with webpack or similar, etc.), the discussion around whether or not to use iFrames etc. would be very different.
The upshot is I think it's OK to go ahead with this proposal, because the dynamic BE plugin loading (and PIP distribution of those) is great in any case. It may turn out that the FE-assembly questions here will be obviated by a later design of broader scope; let's give that the attention it deserves in a separate RFC.
|
Due to uncertainty with the Colab + iframe story, we are going to push out the deadline few days til May 6th. Thanks for all the comments so far! |
|
Sorry for the delay in closing the RFC. Prototype with Colab took longer than expected but everything is more or less looks good now. I have updated the proposal accordingly (the iframe based solution wins). Since there was no major disagreement generally and especially since there wasn't much disagreement on how plugins are loaded, where we spent extra time prototyping with Colab, we will bake the proposal few more days and try to close it on May 19th. Thanks! |
|
Alright, we have let it bake for awhile and I think it is good to close this. |
|
Please push an update changing the status from Proposed to Accepted, and I will merge. Many thanks! |
This RFC proposes a friction-free approach to enabling TensorBoard contributors to create and share TensorBoard plugins as installable extensions. TensorBoard is composed of various plugins that show up as separate dashboards to users (e.g., scalars, graphs, hparams, histograms, etc.). Today. anyone can contribute a new plugin by forking TensorBoard, adding their plugin, and getting the PR merged. However, this can require a lengthy review to make sure it is architected correctly and is aligned with TensorBoard’s goals. This RFC covers two changes that will make this process easier: Plugins can be distributed as Python packages. Users can install them as extensions to TensorBoard. Plugins are no longer limited to use Polymer but can instead use any frontend library or framework (e.g., React) The intention is to enable any prospective contributor to easily be able to create or reuse a visualization with tools familiar to them, and enable users to try it out quickly. Plugins that are useful to many users should still be added to TensorBoard, but this removes the friction needed to start getting feedback or trying out something new.
|
@ewilderj done! Thanks a lot! |
ewilderj
added
RFC: Accepted
This is the change based on tensorflow/community#90. The change makes two improvements: - allow dynamic plugins (as spec in the RFC; uses entry_points to discover) to be loaded - change plugins_listing contract to include metadata about plugins including their respective FE entry point
Comment period ends 2019-05-19
TensorBoard: Improved Plugin Extensibility
This RFC proposes a friction-free approach to enabling TensorBoard contributors to create and share TensorBoard plugins as installable extensions.
TensorBoard is composed of various plugins that show up as separate dashboards to users (e.g., scalars, graphs, hparams, histograms, etc.). Today. anyone can contribute a new plugin by forking TensorBoard, adding their plugin, and getting the PR merged. However, this can require a lengthy review to make sure it is architected correctly and is aligned with TensorBoard’s goals.
This RFC covers two changes that will make this process easier:
Plugins can be distributed as Python packages. Users can install them as extensions to TensorBoard.
Plugins are no longer limited to use Polymer but can instead use any frontend library or framework (e.g., React)
The intention is to enable any prospective contributor to easily be able to create or reuse a visualization with tools familiar to them, and enable users to try it out quickly. Plugins that are useful to many users should still be added to TensorBoard, but this removes the friction needed to start getting feedback or trying out something new.