Skip to content

Commit

Permalink
graph: fix spurious error modal on page load (tensorflow#2651)
Browse files Browse the repository at this point in the history
Summary:
Fixes tensorflow#2646, and fixes tensorflow#2650 as a side effect.

I’ve gone with an additional boolean field `_datasetsFetched` rather
than making `_datasets` nullable because `_datasets` is also passed down
directly to `tf-graph-controls`, which expects a non-null array. This
seemed simpler, all things considered.

Test Plan:

 1. Launch TensorBoard. Navigate to <http://localhost:6006/#graphs>.
 2. Observe that there is no flash of “No graph definition files were
    found” (though there is instead a brief flash of white).
 3. Note that the list of graphs loads, the first graph is selected, and
    the fragment updates.
 4. Refresh the page. Note that the page behaves as in the initial load:
    in particular, there is no error modal.
 5. Change the fragment such that the run is invalid (`#graphs&run=bad`)
    and note that the error modal appears without refreshing the page.
 6. Refresh the page, and note that the error modal appears, and the
    first graph is automatically selected and loaded underneath it.

wchargin-branch: graph-fix-error-modal
  • Loading branch information
wchargin committed Sep 12, 2019
1 parent 30dd2e2 commit f1aa5b8
Showing 1 changed file with 30 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@
<dom-module id="tf-graph-dashboard">
<template>
<paper-dialog id="error-dialog" with-backdrop></paper-dialog>
<template is="dom-if" if="[[_datasetsEmpty(_datasets)]]">
<template
is="dom-if"
if="[[_datasetsState(_datasetsFetched, _datasets, 'EMPTY')]]"
>
<div style="max-width: 540px; margin: 80px auto 0 auto;">
<h3>No graph definition files were found.</h3>
<p>
Expand Down Expand Up @@ -75,7 +78,10 @@ <h3>No graph definition files were found.</h3>
</p>
</div>
</template>
<template is="dom-if" if="[[!_datasetsEmpty(_datasets)]]">
<template
is="dom-if"
if="[[!_datasetsState(datasetsFetched, _datasets, 'PRESENT')]]"
>
<tf-dashboard-layout>
<tf-graph-controls
id="controls"
Expand Down Expand Up @@ -191,6 +197,10 @@ <h3>No graph definition files were found.</h3>
type: Array,
value: () => [],
},
_datasetsFetched: {
type: Boolean,
value: false,
},
_selectedDataset: {
type: Number,
value: 0,
Expand Down Expand Up @@ -275,8 +285,8 @@ <h3>No graph definition files were found.</h3>
'_maybeFetchHealthPills(_debuggerDataEnabled, allStepsModeEnabled, ' +
'specificHealthPillStep, _selectedNode)',
'_maybeInitializeDashboard(_isAttached)',
'_determineSelectedDataset(_datasets, run)',
'_updateSelectedDatasetName(_datasets, _selectedDataset)',
'_determineSelectedDataset(_datasetsFetched, _datasets, run)',
'_updateSelectedDatasetName(_datasetsFetched, _datasets, _selectedDataset)',
],
attached: function() {
this.set('_isAttached', true);
Expand Down Expand Up @@ -366,7 +376,7 @@ <h3>No graph definition files were found.</h3>
this._debuggerDataEnabled &&
this.healthPillsToggledOn &&
this._renderHierarchy &&
!this._datasetsEmpty(this._datasets)
this._datasetsState(this._datasetsFetched, this._datasets, 'PRESENT')
);
},
_maybeInitializeDashboard: function(isAttached) {
Expand Down Expand Up @@ -421,9 +431,10 @@ <h3>No graph definition files were found.</h3>

return {name: runName, tags: tagsWithV1Graph};
});
this._datasetsFetched = true;
});
},
_determineSelectedDataset(datasets, run) {
_determineSelectedDataset(datasetsFetched, datasets, run) {
// By default, load the first dataset.
if (!run) {
// By default, load the first dataset.
Expand All @@ -434,17 +445,20 @@ <h3>No graph definition files were found.</h3>
// If the URL specifies a dataset, load it.
const dataset = datasets.findIndex((d) => d.name === run);
if (dataset === -1) {
// Tell the user if the dataset cannot be found to avoid misleading
// the user.
const dialog = this.$$('#error-dialog');
dialog.textContent = `No dataset named "${run}" could be found.`;
dialog.open();
if (datasetsFetched) {
// Tell the user if the dataset cannot be found to avoid misleading
// the user.
const dialog = this.$$('#error-dialog');
dialog.textContent = `No dataset named "${run}" could be found.`;
dialog.open();
}
return;
}

this.set('_selectedDataset', dataset);
},
_updateSelectedDatasetName(datasets, selectedDataset) {
_updateSelectedDatasetName(datasetsFetched, datasets, selectedDataset) {
if (!datasetsFetched) return;
// Cannot update `run` to update the hash in case datasets for graph is empty.
if (datasets.length <= selectedDataset) return;
this.set('run', datasets[selectedDataset].name);
Expand Down Expand Up @@ -528,8 +542,10 @@ <h3>No graph definition files were found.</h3>
}.bind(this)
);
},
_datasetsEmpty: function(datasets) {
return !datasets || !datasets.length;
_datasetsState: function(datasetsFetched, datasets, state) {
if (!datasetsFetched) return state === 'NOT_LOADED';
if (!datasets || !datasets.length) return state === 'EMPTY';
return state === 'PRESENT';
},
_renderHierarchyChanged: function(renderHierarchy) {
// Reload any data on the graph when the render hierarchy (which determines which nodes are
Expand Down

0 comments on commit f1aa5b8

Please sign in to comment.