Skip to content

Commit

Permalink
timeseris: fix tags not fetching issue when loading (#5300)
Browse files Browse the repository at this point in the history
Previously, we were only waiting for navigation and plugin change events
none of which is useful when freshly loading dashboard since
`activePlugin` will be null until we can fetch the plugins listing at
least once. This change, to fix the problem, also listens to plugins
listing loaded event to assert and fetch the data.
  • Loading branch information
stephanwlee authored Sep 9, 2021
1 parent 7f6ffe3 commit bd83fe8
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 1 deletion.
9 changes: 8 additions & 1 deletion tensorboard/webapp/metrics/effects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,17 @@ export class MetricsEffects implements OnInitEffects {
*
* [Metrics Effects] Init - the initial `activePlugin` is set.
* [Core] Plugin Changed - subsequent `activePlugin` updates.
* [Core] PluginListing Fetch Successful - list of plugins fetched and the
* first `activePlugin` set.
* [App Routing] Navigated - experiment id updates.
*/
private readonly dashboardShownWithoutData$ = this.actions$.pipe(
ofType(initAction, coreActions.changePlugin, routingActions.navigated),
ofType(
initAction,
coreActions.changePlugin,
coreActions.pluginsListingLoaded,
routingActions.navigated
),
withLatestFrom(
this.store.select(getActivePlugin),
this.store.select(getMetricsTagMetadataLoadState)
Expand Down
87 changes: 87 additions & 0 deletions tensorboard/webapp/metrics/effects/metrics_effects_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ import {
} from '../testing';
import {CardId, TooltipSort} from '../types';
import {CardFetchInfo, MetricsEffects, TEST_ONLY} from './index';
import {RouteKind} from '../../app_routing/types';
import {LoadingMechanismType} from '../../types/api';

describe('metrics effects', () => {
let dataSource: MetricsDataSource;
Expand Down Expand Up @@ -171,6 +173,91 @@ describe('metrics effects', () => {
]);
});

it('loads TagMetadata when navigating to a new route', () => {
store.overrideSelector(selectors.getExperimentIdsFromRoute, ['']);
store.overrideSelector(getMetricsTagMetadataLoadState, {
state: DataLoadState.NOT_LOADED,
lastLoadedTimeInMs: null,
});
store.overrideSelector(getActivePlugin, null);
store.refreshState();

actions$.next(buildNavigatedAction({routeKind: RouteKind.EXPERIMENT}));
expect(fetchTagMetadataSpy).not.toHaveBeenCalled();

actions$.next(coreActions.pluginsListingRequested());
store.overrideSelector(getActivePlugin, METRICS_PLUGIN_ID);
store.refreshState();
actions$.next(
coreActions.pluginsListingLoaded({
plugins: {
[METRICS_PLUGIN_ID]: {
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.NG_COMPONENT,
},
disable_reload: true,
tab_name: 'hello',
remove_dom: true,
},
},
})
);

expect(fetchTagMetadataSpy).toHaveBeenCalledTimes(1);
fetchTagMetadataSubject.next(buildDataSourceTagMetadata());
expect(actualActions).toEqual([
actions.metricsTagMetadataRequested(),
actions.metricsTagMetadataLoaded({
tagMetadata: buildDataSourceTagMetadata(),
}),
]);
});

it('does not fetch TagMetadata if default plugin is not timeseries', () => {
store.overrideSelector(selectors.getExperimentIdsFromRoute, ['']);
store.overrideSelector(getMetricsTagMetadataLoadState, {
state: DataLoadState.NOT_LOADED,
lastLoadedTimeInMs: null,
});
store.overrideSelector(getActivePlugin, null);
store.refreshState();

actions$.next(buildNavigatedAction({routeKind: RouteKind.EXPERIMENT}));
expect(fetchTagMetadataSpy).not.toHaveBeenCalled();

actions$.next(coreActions.pluginsListingRequested());
store.overrideSelector(getActivePlugin, 'foo');
store.refreshState();
actions$.next(
coreActions.pluginsListingLoaded({
plugins: {
foo: {
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.NG_COMPONENT,
},
disable_reload: true,
tab_name: 'hello',
remove_dom: true,
},
[METRICS_PLUGIN_ID]: {
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.NG_COMPONENT,
},
disable_reload: true,
tab_name: 'hello',
remove_dom: true,
},
},
})
);

expect(fetchTagMetadataSpy).not.toHaveBeenCalled();
expect(actualActions).toEqual([]);
});

it('does not fetch TagMetadata if data was loaded when opening', () => {
store.overrideSelector(getMetricsTagMetadataLoadState, {
state: DataLoadState.LOADED,
Expand Down

0 comments on commit bd83fe8

Please sign in to comment.