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 startup URL omits trailing slash #1176

Closed
lcswillems opened this issue May 11, 2018 · 6 comments · Fixed by #2101
Closed

--path_prefix startup URL omits trailing slash #1176

lcswillems opened this issue May 11, 2018 · 6 comments · Fixed by #2101

Comments

@lcswillems
Copy link

lcswillems commented May 11, 2018

When I execute tensorboard --logdir storage/logs --path_prefix /test, I get this message: TensorBoard 1.8.0 at http://lcswillems-ubuntu:6006/test (Press CTRL+C to quit). But when I click on the link, I get Not found page.

I precise that when I execute tensorboard --logdir storage/logs, it is working.

I use Tensorboard 1.8.0.

@lcswillems
Copy link
Author

I solved this by replacing /test by /test/ in the command. But there is an issue with the description of the --path_prefix argument. It is written:

A leading slash is required when specifying the path_prefix, however trailing slashes can be omitted.

@nfelt
Copy link
Contributor

nfelt commented May 11, 2018

Hm, what happens if you run tensorboard --logdir storage/logs --path_prefix /test but then visit the URL with a trailing slash, i.e. http://lcswillems-ubuntu:6006/test/? If that works then it's likely just an issue with the URL that's being printed out, not with TensorBoard's actual URL handling for the path prefix.

@lcswillems
Copy link
Author

lcswillems commented May 11, 2018

It is working if I go to http://lcswillems-ubuntu:6006/test/. Thank you for help!

@nfelt
Copy link
Contributor

nfelt commented May 11, 2018

Thanks for checking - in that case I think it's just the logic for printing the URL out that would need a fix.

@nfelt nfelt changed the title path_prefix not working Startup URL omits trailing slash when using --path_prefix May 11, 2018
@nfelt nfelt changed the title Startup URL omits trailing slash when using --path_prefix --path_prefix startup URL omits trailing slash May 11, 2018
@zhangchen-qinyinghua
Copy link

#1620

This mistake is in deed trival. Someone please just correct it.

@thealphacod3r
Copy link
Contributor

@nfelt @wchargin i fixed the ommiting trail slash bug please review this!!

wchargin added a commit that referenced this issue Oct 6, 2019
Summary:
When using a `--path_prefix`, TensorBoard has historically required a
trailing slash after that prefix: e.g., one visits `localhost:6006/foo/`
rather than `localhost:6006/foo`. (See, for instance, #1176.) Different
versions of TensorBoard have different failure modes when the non-slash
path is loaded; currently, the TensorBoard shell loads, but the frontend
computes relative URLs incorrectly. This commit adds a redirect from the
empty path to `/` to avoid the problem.

This logic could be inlined into the `PathPrefixMiddleware`, but we’ll
soon have other middlewares with similar needs, so providing this as its
own middleware avoids duplicating the functionality.

Test Plan:
Launch TensorBoard with `--path_prefix /foo` (or `--path_prefix /foo/`;
the two are equivalent) and navigate to `/foo/` and `/foo` in a browser.
Note that they now both work and resolve to `/foo/`, while prior to this
commit navigating to `/foo` yielded a broken TensorBoard.

wchargin-branch: empty-path-redirect
wchargin-source: 300682590a9a44f1b37d16168b457b4f5898463c
wchargin added a commit that referenced this issue Oct 7, 2019
Summary:
When using a `--path_prefix`, TensorBoard has historically required a
trailing slash after that prefix: e.g., one visits `localhost:6006/foo/`
rather than `localhost:6006/foo`. (See, for instance, #1176.) Different
versions of TensorBoard have different failure modes when the non-slash
path is loaded; currently, the TensorBoard shell loads, but the frontend
computes relative URLs incorrectly. This commit adds a redirect from the
empty path to `/` to avoid the problem.

This logic could be inlined into the `PathPrefixMiddleware`, but we’ll
soon have other middlewares with similar needs, so providing this as its
own middleware avoids duplicating the functionality.

Test Plan:
Launch TensorBoard with `--path_prefix /foo` (or `--path_prefix /foo/`;
the two are equivalent) and navigate to `/foo/` and `/foo` in a browser.
Note that they now both work and resolve to `/foo/`, while prior to this
commit navigating to `/foo` yielded a broken TensorBoard.

wchargin-branch: empty-path-redirect
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants