Skip to content

Allow TensorBoard to load despite existing MIME type misconfiguration #16203

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 1 commit into from
May 11, 2021

Conversation

joyceerhl
Copy link

@joyceerhl joyceerhl commented May 11, 2021

For #16072

This comprehensively solves bugs like #15636 (comment) without us having to tell users how to delete problematic MIME types from the Windows registry like https://github.com/microsoft/vscode-python/wiki/Troubleshooting-TensorBoard-integration-in-VS-Code#integrated-tensorboard-sessions

In the screenshot below, you can see that even if I've intentionally misconfigured '.js' files to resolve to a MIME type other than 'application/javascript', TensorBoard is still able to load:

image

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • The wiki is updated with any design decisions/details.

@joyceerhl joyceerhl added the skip tests Updates to tests unnecessary label May 11, 2021
@joyceerhl joyceerhl marked this pull request as ready for review May 11, 2021 06:03
os.environ["VSCODE_TENSORBOARD_LAUNCH"] = "1"

# Work around incorrectly configured MIME types on Windows
mimetypes.add_type("application/javascript", ".js")
Copy link

Choose a reason for hiding this comment

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

What happens if this is already there? Does mimetypes just ignore the call?

Copy link
Author

@joyceerhl joyceerhl May 11, 2021

Choose a reason for hiding this comment

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

From https://docs.python.org/3/library/mimetypes.html, "When the extension is already known, the new type will replace the old one. When the type is already known the extension will be added to the list of known extensions."

i.e. if .js is mapped to something else, the existing type will be overwritten by what we specify. For context, TensorBoard has done the exact same thing in their CLI launcher codepath for over a year: https://github.com/tensorflow/tensorboard/blob/ea18bb76ebd515f80a7d99656bb08c045638a991/tensorboard/program.py#L392 We don't benefit from that code because we're launching from their Python API entrypoint, namely the launch() function, so this PR just applies the fix they already have to our codepath as well.

@joyceerhl joyceerhl merged commit c983c03 into main May 11, 2021
@joyceerhl joyceerhl deleted the dev/joyceerhl/tensorboard-mimetypes branch May 11, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip tests Updates to tests unnecessary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants