Skip to content
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.

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

Add plugin to redirect users to new profile plugin #3216

Merged
merged 6 commits into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions tensorboard/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ py_library(
"//tensorboard/plugins/mesh:mesh_plugin",
"//tensorboard/plugins/pr_curve:pr_curves_plugin",
"//tensorboard/plugins/profile:profile_plugin_loader",
"//tensorboard/plugins/profile_redirect:profile_redirect_plugin",
"//tensorboard/plugins/scalar:scalars_plugin",
"//tensorboard/plugins/text:text_plugin",
],
Expand Down
1 change: 1 addition & 0 deletions tensorboard/components/tf_tensorboard/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ tf_web_library(
"//tensorboard/plugins/mesh/tf_mesh_dashboard",
"//tensorboard/plugins/pr_curve/tf_pr_curve_dashboard",
"//tensorboard/plugins/profile/tf_profile_dashboard",
"//tensorboard/plugins/profile_redirect/tf_profile_redirect_dashboard",
"//tensorboard/plugins/scalar/tf_scalar_dashboard",
"//tensorboard/plugins/text/tf_text_dashboard",
],
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/components/tf_tensorboard/default-plugins.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
<link rel="import" href="../tf-text-dashboard/tf-text-dashboard.html" />
<link rel="import" href="../tf-pr-curve-dashboard/tf-pr-curve-dashboard.html" />
<link rel="import" href="../tf-profile-dashboard/tf-profile-dashboard.html" />
<link
rel="import"
href="../tf-profile-redirect-dashboard/tf-profile-redirect-dashboard.html"
/>
<link rel="import" href="../tf-beholder-dashboard/tf-beholder-dashboard.html" />
<link
rel="import"
Expand Down
2 changes: 2 additions & 0 deletions tensorboard/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
)
from tensorboard.plugins.pr_curve import pr_curves_plugin
from tensorboard.plugins.profile import profile_plugin_loader
from tensorboard.plugins.profile_redirect import profile_redirect_plugin
from tensorboard.plugins.scalar import scalars_plugin
from tensorboard.plugins.text import text_plugin
from tensorboard.plugins.mesh import mesh_plugin
Expand All @@ -74,6 +75,7 @@
text_plugin.TextPlugin,
pr_curves_plugin.PrCurvesPlugin,
profile_plugin_loader.ProfilePluginLoader,
profile_redirect_plugin.ProfileRedirectPluginLoader,
beholder_plugin_loader.BeholderPluginLoader,
interactive_inference_plugin_loader.InteractiveInferencePluginLoader,
hparams_plugin.HParamsPlugin,
Expand Down
28 changes: 28 additions & 0 deletions tensorboard/plugins/profile_redirect/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Description:
# Plugin with installation instructions for dynamic profile plugin

package(default_visibility = ["//tensorboard:internal"])

licenses(["notice"]) # Apache 2.0

exports_files(["LICENSE"])

py_library(
name = "profile_redirect_plugin",
srcs = ["profile_redirect_plugin.py"],
srcs_version = "PY2AND3",
deps = [
"//tensorboard/plugins:base_plugin",
],
)

py_test(
name = "profile_redirect_plugin_test",
srcs = ["profile_redirect_plugin_test.py"],
srcs_version = "PY2AND3",
deps = [
":profile_redirect_plugin",
"//tensorboard:test",
"//tensorboard/plugins:base_plugin",
],
)
14 changes: 14 additions & 0 deletions tensorboard/plugins/profile_redirect/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Copyright 2020 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.
# ==============================================================================
55 changes: 55 additions & 0 deletions tensorboard/plugins/profile_redirect/profile_redirect_plugin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Copyright 2020 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.
# ==============================================================================
"""Plugin that only displays a message with installation instructions."""

from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

from tensorboard.plugins import base_plugin


class ProfileRedirectPluginLoader(base_plugin.TBLoader):
"""Load the redirect notice iff the dynamic plugin is unavailable."""

def load(self, context):
try:
import tensorboard_plugin_profile

# If we successfully load the dynamic plugin, don't show
# this redirect plugin at all.
return None
except ImportError:
return _ProfileRedirectPlugin(context)


class _ProfileRedirectPlugin(base_plugin.TBPlugin):
"""Redirect notice pointing users to the new dynamic profile plugin."""

plugin_name = "profile_redirect"

def get_plugin_apps(self):
return {}

def is_active(self):
return False

def frontend_metadata(self):
return base_plugin.FrontendMetadata(
element_name="tf-profile-redirect-dashboard",
# TODO(@wchargin): When the static profile plugin is removed
# in favor of the dynamic one, remove this parenthetical.
tab_name="Profile (moved)",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# Copyright 2020 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.
# ==============================================================================
"""Tests for `profile_redirect_plugin`."""

from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

import contextlib
import sys
from unittest import mock

from tensorboard.plugins import base_plugin
from tensorboard.plugins.profile_redirect import profile_redirect_plugin
from tensorboard import test as tb_test


_DYNAMIC_PLUGIN_MODULE = "tensorboard_plugin_profile"


class ProfileRedirectPluginLoaderTest(tb_test.TestCase):
"""Tests for `ProfileRedirectPluginLoader`."""

def test_loads_when_no_dynamic_plugin(self):
with contextlib.ExitStack() as stack:
stack.enter_context(mock.patch.dict(sys.modules))
sys.modules.pop(_DYNAMIC_PLUGIN_MODULE, None)

real_import = __import__

def fake_import(name, *args, **kwargs):
if name == _DYNAMIC_PLUGIN_MODULE:
raise ImportError("Pretend I'm not here")
else:
return real_import(name, *args, **kwargs)

stack.enter_context(mock.patch("builtins.__import__", fake_import))

plugin_class = profile_redirect_plugin._ProfileRedirectPlugin
plugin_init = stack.enter_context(
mock.patch.object(plugin_class, "__init__", return_value=None)
)

loader = profile_redirect_plugin.ProfileRedirectPluginLoader()
context = base_plugin.TBContext()
result = loader.load(context)
self.assertIsInstance(result, plugin_class)
plugin_init.assert_called_once_with(context)

def test_does_not_load_when_dynamic_plugin_present(self):
with contextlib.ExitStack() as stack:
stack.enter_context(mock.patch.dict(sys.modules))
sys.modules.pop(_DYNAMIC_PLUGIN_MODULE, None)

real_import = __import__

def fake_import(name, *args, **kwargs):
if name == _DYNAMIC_PLUGIN_MODULE:
arbitrary_module = sys
sys.modules.setdefault(
_DYNAMIC_PLUGIN_MODULE, arbitrary_module
)
return arbitrary_module
else:
return real_import(name, *args, **kwargs)

stack.enter_context(mock.patch("builtins.__import__", fake_import))

plugin_class = profile_redirect_plugin._ProfileRedirectPlugin
stephanwlee marked this conversation as resolved.
Show resolved Hide resolved
plugin_init = stack.enter_context(
mock.patch.object(plugin_class, "__init__", return_value=None)
)

loader = profile_redirect_plugin.ProfileRedirectPluginLoader()
context = base_plugin.TBContext()
result = loader.load(context)
self.assertIsNone(result)
plugin_init.assert_not_called()


if __name__ == "__main__":
tb_test.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package(default_visibility = ["//tensorboard:internal"])

load("//tensorboard/defs:web.bzl", "tf_web_library")

licenses(["notice"]) # Apache 2.0

tf_web_library(
name = "tf_profile_redirect_dashboard",
srcs = ["tf-profile-redirect-dashboard.html"],
path = "/tf-profile-redirect-dashboard",
deps = [
"//tensorboard/components/tf_imports:polymer",
"@org_polymer_paper_button",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
<!--
@license
Copyright 2020 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.
-->

<link rel="import" href="../paper-button/paper-button.html" />
<link rel="import" href="../tf-imports/polymer.html" />

<!--
A frontend that directs users to upgrade to the new version of the
profile plugin, now distributed separately from TensorBoard.
-->
<dom-module id="tf-profile-redirect-dashboard">
<template>
<div class="message">
<h3>The profile plugin has moved.</h3>
<p>
Please install the new version of the profile plugin from PyPI by
running the following command from the machine running TensorBoard:
</p>
<textarea id="commandTextarea" readonly on-blur="_removeCopiedMessage">
[[_installCommand]]</textarea
>
<div id="copyContainer">
<span id="copiedMessage"></span>
<paper-button raised on-tap="_copyInstallCommand"
>Copy to clipboard</paper-button
>
</div>
</div>

<style include="dashboard-style"></style>
Copy link
Contributor

Choose a reason for hiding this comment

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

Omit this unless you are making use of it. Else, I believe you need to import https://github.com/tensorflow/tensorboard/blob/master/tensorboard/components/tf_dashboard_common/dashboard-style.html in rel="import".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks! (I’d used this originally but decided that I didn’t need
it; removed the build dep and import, but forgot to remove this include.
Done.)

<style>
.message {
margin: 80px auto 0 auto;
max-width: 540px;
}
#commandTextarea {
margin-top: 1ex;
padding: 1ex 0;
resize: vertical;
width: 100%;
}
#copyContainer {
display: flex;
}
#copiedMessage {
align-self: center;
flex-grow: 1;
font-style: italic;
padding-right: 1em;
text-align: right;
}
</style>
</template>

<script>
(function() {
Polymer({
is: 'tf-profile-redirect-dashboard',
properties: {
_installCommand: {
type: String,
readonly: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

readonly -> readOnly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch; thanks! Done.

value: [
'pip install --extra-index-url https://test.pypi.org/simple/ \\',
' tensorboard_plugin_profile',
].join('\n'),
},
},
_copyInstallCommand() {
const tryCopy = (async () => {
const textarea = this.$.commandTextarea;
textarea.select();
try {
await navigator.clipboard.writeText(textarea.textContent);
stephanwlee marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
// Fallback approach.
if (!document.execCommand('copy')) {
return Promise.reject();
stephanwlee marked this conversation as resolved.
Show resolved Hide resolved
}
}
})();
tryCopy
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: since we are only "officially supporting" evergreen browsers, you may use async/await and make this code read better (wait... you are already using async/await :)).

async _copyInstallCommand() {
    const tryCopy = async () => {
      const textarea = this.$.commandTextarea;
      textarea.select();
      try {
        await navigator.clipboard.writeText(textarea.textContent);
      } catch (error) {
        // Fallback approach.
        if (!document.execCommand('copy')) {
          return Promise.reject();
        }
      }
    };

    try {
      await tryCopy();
      this.$.copiedMessage.innerText = 'Copied.';
    }  catch (e) {
      this.$.copiedMessage.innerText = 'Failed to copy to clipboard..';
    }
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that second try structure is definitely nicer. Done; thanks!

.then(() => {
this.$.copiedMessage.innerText = 'Copied.';
})
.catch(() => {
this.$.copiedMessage.innerText = 'Failed to copy to clipboard.';
});
},
_removeCopiedMessage() {
this.$.copiedMessage.innerText = '';
},
});
})();
</script>
</dom-module>