Remove the demoify function (#490)

Summary:
We don’t support the demo mode at all, nor do we have any plans to. This
function is complicating our logic and getting in the way of things that
we actually want to do, so out it goes.

Note that this commit does not remove “demo mode” entirely: the router
still has `demoMode`, and I didn’t go through and remove all of its
uses. (But that’s not a bad idea, and there are only a handful.)

Test Plan:
I tested that the image and audio dashboards still properly load images
and audio. Changes to the test are not tested, as we don’t run tests.

wchargin-branch: remove-demoify

Author: wchargin

tensorboard/components/tf_backend/backend.ts

@@ -16,7 +16,6 @@ limitations under the License.
 import {compareTagNames} from '../vz-sorting/sorting.js';
 import {RequestManager} from './requestManager.js';
 import {getRouter} from './router.js';
-import {demoify, queryEncoder} from './urlPathHelpers.js';
 
 export type RunToTag = {
   [run: string]: string[];

tensorboard/components/tf_backend/router.ts

@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 ==============================================================================*/
 
-import {demoify, queryEncoder} from './urlPathHelpers.js';
+import {queryEncoder} from './urlPathHelpers.js';
 
 export type RunTagUrlFn = (tag: string, run: string) => string;
 
@@ -35,19 +35,13 @@ export interface Router {
  * @param demoMode {boolean} Whether to modify urls for filesystem demo usage.
  */
 export function createRouter(dataDir = 'data', demoMode = false): Router {
-  var clean = demoMode ? demoify : (x) => x;
   if (dataDir[dataDir.length - 1] === '/') {
     dataDir = dataDir.slice(0, dataDir.length - 1);
   }
   function standardRoute(route: string, demoExtension = '.json'):
       ((tag: string, run: string) => string) {
     return function(tag: string, run: string): string {
-      var url =
-          dataDir + '/' + route + clean(queryEncoder({tag: tag, run: run}));
-      if (demoMode) {
-        url += demoExtension;
-      }
-      return url;
+      return dataDir + '/' + route + queryEncoder({tag: tag, run: run});
     };
   }
   function pluginRoute(pluginName: string, route: string): string {
@@ -56,7 +50,7 @@ export function createRouter(dataDir = 'data', demoMode = false): Router {
   function pluginRunTagRoute(pluginName: string, route: string):
       ((tag: string, run: string) => string) {
     const base = pluginRoute(pluginName, route);
-    return (tag, run) => base + clean(queryEncoder({tag, run}));
+    return (tag, run) => base + queryEncoder({tag, run});
   }
   return {
     logdir: () => dataDir + '/logdir',

tensorboard/components/tf_backend/test/backendTests.ts

@@ -16,24 +16,13 @@ limitations under the License.
 import {filterTags, getRuns, getTags, RunToTag, TYPES} from '../backend.js';
 import {RequestManager} from '../requestManager.js';
 import {createRouter, setRouter} from '../router.js';
-import {BAD_CHARACTERS, demoify, queryEncoder} from '../urlPathHelpers.js';
+import {queryEncoder} from '../urlPathHelpers.js';
 
 describe('urlPathHelpers', () => {
-  it('demoify works as expected', () => {
-    const demoified = demoify(BAD_CHARACTERS);
-    let allClean = '';
-    for (let i = 0; i < BAD_CHARACTERS.length; i++) {
-      allClean += '_';
-    }
-    chai.assert.equal(demoified, allClean, 'cleaning the BAD_CHARACTERS works');
-    chai.assert.equal(demoify('foozod'), 'foozod', 'doesnt change safe string');
-    chai.assert.equal(demoify('foo zod (2)'), 'foo_zod__2_', 'simple case');
-  });
-
-  it('queryEncoder works with demoify on spaces and parens', () => {
+  it('queryEncoder works on spaces and parens', () => {
     const params = {foo: 'something with spaces and (parens)'};
-    const actual = demoify(queryEncoder(params));
-    const expected = '_foo_something_with_spaces_and__28parens_29';
+    const actual = queryEncoder(params);
+    const expected = 'something+with+spaces+and+%28parens%29';
     chai.assert.equal(actual, expected);
   });
 });

tensorboard/components/tf_backend/urlPathHelpers.ts

@@ -12,18 +12,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 ==============================================================================*/
-export const BAD_CHARACTERS = '#%&{}\\/<>*? $!\'":@+`|=() ';
-/** Cleanup a url so that it can be loaded from a filesystem. */
-export function demoify(s) {
-  // for consistency with python's urllib.urlencode
-  s = s.replace(new RegExp('%20', 'g'), '+');
-  for (let i = 0; i < BAD_CHARACTERS.length; i++) {
-    const c = BAD_CHARACTERS[i];
-    s = s.replace(new RegExp('\\' + c, 'g'), '_');
-  }
-  return s;
-}
-
 export function queryEncoder(params?: any): string {
   // It's important that the keys be sorted, so we always grab the right file
   // if we are talking to the backend generated by serialze_tensorboard.py

tensorboard/plugins/audio/tf_audio_dashboard/tf-audio-loader.html

@@ -118,7 +118,6 @@
     import {getRouter} from "../tf-backend/router.js";
     import {formatDate} from '../tf-card-heading/util.js';
     import {runsColorScale} from "../tf-color-scale/colorScale.js";
-    import * as urlPathHelpers from "../tf-backend/urlPathHelpers.js";
 
     Polymer({
       is: "tf-audio-loader",
@@ -235,19 +234,12 @@
           // The route lacks GET parameters. Just use ours.
           query = '?' + query;
         }
-        if (getRouter().isDemoMode()) {
-          query = urlPathHelpers.demoify(query);
-        }
 
-        let individualAudioURL = route + query;
-        if (getRouter().isDemoMode()) {
-          individualAudioURL += '.png';
-        } else {
-          // Include wall_time just to disambiguate the URL and force
-          // the browser to reload the audio when the URL changes. The
-          // backend doesn't care about the value.
-          individualAudioURL += `&ts=${audioMetadata.wall_time}`;
-        }
+        // Include wall_time just to disambiguate the URL and force
+        // the browser to reload the audio when the URL changes. The
+        // backend doesn't care about the value.
+        const individualAudioURL = `${route}${query}&ts=${audioMetadata.wall_time}`;
+
         return {
           wall_time: formatDate(new Date(audioMetadata.wall_time * 1000)),
           step: audioMetadata.step,

tensorboard/plugins/image/tf_image_dashboard/tf-image-loader.html

@@ -185,7 +185,6 @@
     import {getRouter} from "../tf-backend/router.js";
     import {formatDate} from '../tf-card-heading/util.js';
     import {runsColorScale} from "../tf-color-scale/colorScale.js";
-    import * as urlPathHelpers from "../tf-backend/urlPathHelpers.js";
 
     Polymer({
       is: "tf-image-loader",
@@ -330,19 +329,12 @@
           // The route lacks GET parameters. Just use ours.
           query = '?' + query;
         }
-        if (getRouter().isDemoMode()) {
-          query = urlPathHelpers.demoify(query);
-        }
 
-        let individualImageUrl = route + query;
-        if (getRouter().isDemoMode()) {
-          individualImageUrl += '.png';
-        } else {
-          // Include wall_time just to disambiguate the URL and force
-          // the browser to reload the image when the URL changes. The
-          // backend doesn't care about the value.
-          individualImageUrl += `&ts=${imageMetadata.wall_time}`;
-        }
+        // Include wall_time just to disambiguate the URL and force
+        // the browser to reload the image when the URL changes. The
+        // backend doesn't care about the value.
+        const individualImageUrl = `${route}${query}&ts=${imageMetadata.wall_time}`;
+
         return {
           width: imageMetadata.width,
           height: imageMetadata.height,