backend: always provide data provider on context #3415
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Until now, `context.data_provider` was provided unless `generic_data` was explicitly set to `false`. Now, it’s provided in all cases (except the legacy DB modes). This was always expected to be safe, and has been live by default since TensorBoard 1.15.0. This is needed to write plugins that unconditionally assume that a data provider is available. A consequence of this is that `is_active`’s use of the experimental `list_plugins` method is now always on as well. This has been on by default for two months, but not yet in any release (other than the upcoming TensorBoard 2.2.0), so it’s slightly riskier, but it’s also expected to be safe. Test Plan: Run TensorBoard with `--generic_data false`. If pointing at an empty logdir, no plugins are shown as active. If pointing at a full plugin, all appropriate plugins are shown as active. The debugger plugin is still marked as active if `--debugger_port` is specified and as inactive if no relevant flags are given, so the fallback `is_active` calls are still working. wchargin-branch: backend-always-data-provider
nfelt
approved these changes
Mar 24, 2020
Contributor
nfelt
left a comment
nfelt
left a comment
There was a problem hiding this comment.
Should we just remove the false option to --generic_data at this point? Or at least update the flag description? It currently says Whether to use generic data provider infrastructure but at this point it's more like:
Controls whether plugins should read from the generic data provider infrastructure, if they support both that and the legacy multiplexer API. Some plugins may have only the former or only the latter, in which case they are not affected by this flag. The "auto" option enables this only for plugins that are considered to have stable support for generic data providers.
wchargin-branch: backend-always-data-provider wchargin-source: f3791f9dab87dc891530b8c74e8d89c83f4b3213
wchargin-branch: backend-always-data-provider wchargin-source: f3791f9dab87dc891530b8c74e8d89c83f4b3213
Contributor
Author
|
Good point; updated the description. I’m also open to removing it |
nfelt
approved these changes
Mar 24, 2020
Contributor
nfelt
left a comment
nfelt
left a comment
There was a problem hiding this comment.
Thanks, description LGTM. It's still at least theoretically useful for a few plugins for now so IMO no rush to remove it entirely.
Contributor
Author
|
Also, the graphs dashboard is still not at parity (only shows run-level |
bileschi
pushed a commit
to bileschi/tensorboard
that referenced
this pull request
Apr 15, 2020
Summary: Until now, `context.data_provider` was provided unless `generic_data` was explicitly set to `false`. Now, it’s provided in all cases (except the legacy DB modes). This was always expected to be safe, and has been live by default since TensorBoard 1.15.0. This is needed to write plugins that unconditionally assume that a data provider is available. A consequence of this is that `is_active`’s use of the experimental `list_plugins` method is now always on as well. This has been on by default for two months, but was first released in TensorBoard 2.2.0 (just released), so it’s slightly riskier, but also expected to be safe. Test Plan: Run TensorBoard with `--generic_data false`. If pointing at an empty logdir, no plugins are shown as active. If pointing at a full plugin, all appropriate plugins are shown as active. The debugger plugin is still marked as active if `--debugger_port` is specified and as inactive if no relevant flags are given, so the fallback `is_active` calls are still working. wchargin-branch: backend-always-data-provider
bileschi
pushed a commit
that referenced
this pull request
Apr 15, 2020
Summary: Until now, `context.data_provider` was provided unless `generic_data` was explicitly set to `false`. Now, it’s provided in all cases (except the legacy DB modes). This was always expected to be safe, and has been live by default since TensorBoard 1.15.0. This is needed to write plugins that unconditionally assume that a data provider is available. A consequence of this is that `is_active`’s use of the experimental `list_plugins` method is now always on as well. This has been on by default for two months, but was first released in TensorBoard 2.2.0 (just released), so it’s slightly riskier, but also expected to be safe. Test Plan: Run TensorBoard with `--generic_data false`. If pointing at an empty logdir, no plugins are shown as active. If pointing at a full plugin, all appropriate plugins are shown as active. The debugger plugin is still marked as active if `--debugger_port` is specified and as inactive if no relevant flags are given, so the fallback `is_active` calls are still working. wchargin-branch: backend-always-data-provider
Contributor
Author
|
(In hindsight, maybe |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Until now,
context.data_providerwas provided unlessgeneric_datawas explicitly set to
false. Now, it’s provided in all cases (exceptthe legacy DB modes). This was always expected to be safe, and has been
live by default since TensorBoard 1.15.0.
This is needed to write plugins that unconditionally assume that a data
provider is available.
A consequence of this is that
is_active’s use of the experimentallist_pluginsmethod is now always on as well. This has been on bydefault for two months, but was first released in TensorBoard 2.2.0
(just released), so it’s slightly riskier, but also expected to be safe.
Test Plan:
Run TensorBoard with
--generic_data false. If pointing at an emptylogdir, no plugins are shown as active. If pointing at a full plugin,
all appropriate plugins are shown as active. The debugger plugin is
still marked as active if
--debugger_portis specified and as inactiveif no relevant flags are given, so the fallback
is_activecalls arestill working.
wchargin-branch: backend-always-data-provider