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

--path_prefix broken in 2.4.0 #4421

Closed
qrtt1 opened this issue Dec 3, 2020 · 4 comments
Closed

--path_prefix broken in 2.4.0 #4421

qrtt1 opened this issue Dec 3, 2020 · 4 comments

Comments

@qrtt1
Copy link

qrtt1 commented Dec 3, 2020

Describe the current behavior

Some tensorboards installed with tensorflow couldn't access the right URLs.

Reproducible steps

  1. install a tf 2.3.1
  2. start tensorboard with prefix
tensorboard --logdir logs --port 5001 --bind_all  --path_prefix /abc/def

We got failed to fetch runs

image

Because some requests did not add abc/def prefix to the base URL:

image

Describe the expected behavior

All tensorboard requests should add prefix to the base URL if --path_prefix has given.

Standalone code to reproduce the issue

I created a regression tests project to investigate the problem

https://github.com/qrtt1/tensorboard-regression/runs/1487646153

It got worse after tf 2.3.0 (they use tensorboard 2.4.0):

image

@stephanwlee
Copy link
Contributor

stephanwlee commented Dec 3, 2020

Thanks for the report. I was able to reproduce it and it is a serious regression. It is quite interesting to see a puppeteer based tests that catches regression for TensorBoard :)

For TensorBoarder:

This change is related to the router in the 2.4.0. In the release, we introduced a handrolled router which is unaware of notion of path prefix and eagerly redirects "/foo/bar/index.html" to "/" which breaks all the subsequent requests. Few ways to move forward:

  • make the request to "/environments" before bootstrapping components to fetch the path_prefix
    • this is a bit nasty because environment has an experiment specific data_location. I think we should disentangle global setting vs. per-exp settings
    • period of blank page until the request is successful
  • invest in the dehydration/rehydration scheme to populate some state when flushing down the HTML
  • use cookie/query-param or whatever to achieve the same effect of dehydration

@MrBago
Copy link

MrBago commented Jan 13, 2021

@stephanwlee I just ran into this and it's nice to see that you just merged #4423, thank you! Any chance we might be able to get a patch release of 2.4 with this fix backported?

@wchargin
Copy link
Contributor

We do plan to publish a patch release. I’ve filed #4547 to track it.

@wchargin
Copy link
Contributor

TensorBoard 2.4.1 shipped with this patch.

@wchargin wchargin changed the title --path_prefix broken in 2.4.0+ --path_prefix broken in 2.4.0 Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants