-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Remove “demo mode” from the router #1640
Labels
Comments
Merged
This was referenced Nov 29, 2018
wchargin
added a commit
that referenced
this issue
Nov 29, 2018
Summary: See #1640. This removes all demos that instantiate a router in demo mode. None of these demos worked: I loaded each one manually, and each one failed to load some required resources. Demos in `plugins/graph` and `plugins/interactive_inference` have been allowed to stay, because (a) they actually work to at least some degree, and (b) they do not use our router’s demo mode so are not posing any obstacles. For ease of review, this commit is a mass-deletion only. A separate commit will include the structural changes that need to be made to actually remove demo-mode. Test Plan: Check that routers are only created in the `tf_backend` implementation and tests: $ git grep --name-only createRouter tensorboard/components/tf_backend/router.ts tensorboard/components/tf_backend/test/backendTests.ts Note that all existing tests pass in Python 2 and 3. wchargin-branch: remove-demos
wchargin
added a commit
that referenced
this issue
Nov 29, 2018
Summary: Resolves #1640. Test Plan: Within TypeScript files, the fact that the code typechecks is sufficient to verify that callers are correct. To account for JS-in-HTML files, first note that `git grep isDemoMode` returns no results. Then note that each call site of `git grep pluginRoute` has been updated to use either the 2-argument or 3-argument form (i.e., not passing a `demoCustomExt`). I also manually checked the basic behavior of each plugin on the frontend. wchargin-branch: router-remove-demo-mode
wchargin
added a commit
that referenced
this issue
Nov 29, 2018
Summary: Resolves #1640. Test Plan: Within TypeScript files, the fact that the code typechecks is sufficient to verify that callers are correct. To account for JS-in-HTML files, first note that `git grep isDemoMode` returns no results. Then note that each call site of `git grep pluginRoute` has been updated to use either the 2-argument or 3-argument form (i.e., not passing a `demoCustomExt`). I also manually checked the basic behavior of each plugin on the frontend. wchargin-branch: router-remove-demo-mode
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
TensorBoard has long had a “demo mode” in its router, and accompanying
entry points like
//tensorboard/components/tf_tensorboard:demo
. Thedemo used to be hosted statically on tensorflow.org, but this hasn’t
been the case for at least 18 months. In fact, our demos have been
completely broken for most of that time (they are broken right now), and
no one has complained. Meanwhile, due to the
demoMode
flag in therouter, the rest of our code has to attempt to accommodate this mode,
even though it never actually runs.
As the demos aren’t providing any value and are making the rest of the
codebase harder to reason about, we should remove them.
If we later want to write unit tests for Polymer components that need to
access some sort of static data, we should attempt to design these
components’ APIs such that the critical parts don’t require network
access, so that the components can be tested as they actually run in
production rather than in a separate “demo mode”. If that is infeasible,
we should consider using Sinon/etc. to mock out the appropriate network
requests. As a last resort, we can always look into reinstating the
demos and actually getting them to work.
The text was updated successfully, but these errors were encountered: