Skip to content

colab: use proxyPort for dynamic plugin #2798

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

Merged
merged 4 commits into from
Oct 22, 2019

Conversation

stephanwlee
Copy link
Contributor

proxyPort is a relatively new API that lets colab proxy a server from a
port to url given by the method. It installs a serviceworker when
invoked so a request is proxied to the correct kernel.

This API makes:

  1. Colab integration simpler
  2. allows an iframe to be nestable and, hence, enable dynamic plugin

With this change, we now removed the colab not supported page.

proxyPort is a relatively new API that lets colab proxy a server from a
port to url given by the method. It installs a serviceworker when
invoked so a request is proxied to the correct kernel.

This API makes:
1. Colab integration simpler
2. allows an iframe to be nestable and, hence, enable dynamic plugin

With this change, we now removed the colab not supported page.
Copy link
Contributor

@wchargin wchargin left a comment

Choose a reason for hiding this comment

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

Awesome!

I seem to remember display.IFrame behaving slightly strangely in the
past, but can’t recall details. If this works for both newly launched
TensorBoards and reused TensorBoards (if display_handle:’s two cases),
then it looks good to me.

Does this fix the profile dashboard, too? Can we remove its special
casing for Colab (tf-profile-dashboard.html)?

@stephanwlee
Copy link
Contributor Author

If this works for both newly launched
TensorBoards and reused TensorBoards (if display_handle:’s two cases),
then it looks good to me.

In my testing, it looked fine but we, during offline discussion, identified potential
user experience regression from using proxyPort. Will hold off merging.

@stephanwlee stephanwlee merged commit a565541 into tensorflow:master Oct 22, 2019
@stephanwlee stephanwlee deleted the colab branch October 22, 2019 21:33
wchargin added a commit that referenced this pull request Sep 23, 2020
Summary:
In #4050, we added multiplexing to scalar chart fetches for performance.
This works nicely in local TensorBoard and public Colab, but the POST
request machinery isn’t yet supported in the Google-internal version of
Colab, so #4050 caused a regression there. This patch conditionally
rolls back the change for Colab only. This includes rolling it back for
public Colab, where it worked fine. We hope that this isn’t too big of a
problem, since we expect that most Colab users are inspecting datasets
with few runs (generated from within Colab) rather than massive
hyperparameter sweeps. We also hope that we can simply revert this patch
once the Google-internal behavior is fixed.

Jupyter environments are unaffected (and still work).

We used to have a global `window.TENSORBOARD_ENV` that listed whether we
were in Colab, but we removed that in #2798 because we thought that it
was no longer necessary. Since in that PR we switched to create an
iframe rather than manually linking and loading the HTML (much nicer, to
be sure), we now plumb this information via a query parameter. This also
has the advantage that it’s easy to test the Colab codepaths by simply
adding that query parameter to a normal TensorBoard instance.

Test Plan:
First, test that normal TensorBoard (`bazel run //tensorboard`) still
works with multiplexing, and that adding `?tensorboardColab=true` causes
it to send multiple GET requests instead. Then, build the Pip package
(`bazel run //tensorboard/pip_package:extract_pip_packages`), upload it
to public Colab, and install it into the runtime. Verify that the scalar
charts still work there, albeit without multiplexing. Finally,
cherry-pick these changes into google3 via a test sync and follow the
test plan at <http://cl/333398676> to verify the fix.

wchargin-branch: scalars-nomux-colab
wchargin-source: 453503f1dea196985e889be483c2a7675cc87aa1
wchargin added a commit that referenced this pull request Sep 24, 2020
Summary:
In #4050, we added multiplexing to scalar chart fetches for performance.
This works nicely in local TensorBoard and public Colab, but the POST
request machinery isn’t yet supported in the Google-internal version of
Colab, so #4050 caused a regression there. This patch conditionally
rolls back the change for Colab only. This includes rolling it back for
public Colab, where it worked fine. We hope that this isn’t too big of a
problem, since we expect that most Colab users are inspecting datasets
with few runs (generated from within Colab) rather than massive
hyperparameter sweeps. We also hope that we can simply revert this patch
once the Google-internal behavior is fixed.

Jupyter environments are unaffected (and still work).

We used to have a global `window.TENSORBOARD_ENV` that listed whether we
were in Colab, but we removed that in #2798 because we thought that it
was no longer necessary. Since in that PR we switched to create an
iframe rather than manually linking and loading the HTML (much nicer, to
be sure), we now plumb this information via a query parameter. This also
has the advantage that it’s easy to test the Colab codepaths by simply
adding that query parameter to a normal TensorBoard instance.

Fixes #4174.

Test Plan:
First, test that normal TensorBoard (`bazel run //tensorboard`) still
works with multiplexing, and that adding `?tensorboardColab=true` causes
it to send multiple GET requests instead. Then, build the Pip package
(`bazel run //tensorboard/pip_package:extract_pip_packages`), upload it
to public Colab, and install it into the runtime. Verify that the scalar
charts still work there, albeit without multiplexing. Finally,
cherry-pick these changes into google3 via a test sync and follow the
test plan at <http://cl/333398676> to verify the fix.

wchargin-branch: scalars-nomux-colab
bmd3k added a commit that referenced this pull request Oct 6, 2020
…4218)

* Motivation for features / changes

  Hparams dashboard does not work in Google-internal colab environments because they do not support POST requests. 

* Technical description of changes

  Reuse mechanism introduced in #4191 that provides a 'tensorboardColab' query parameter in the URL to indicate a colab environment. If the value for the parameter is true then the Hparams dashboard will send GET requests. Otherwise it sends POST requests.

  There already existed code that purported to determine if the hparams dashboard was running in a colab environment but this code has not worked for some time, possibly since #2798. The code in this PR simply replaces that code. The plumbing to pipe this through other layers had already been implemented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants