Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions tensorbored/webapp/routes/dashboard_deeplink_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
SMOOTHING_KEY,
TAG_FILTER_KEY,
} from './dashboard_deeplink_provider_types';
// Note: SMOOTHING_KEY is only used for deserialization (backwards compat with old URLs).
import {featureFlagsToSerializableQueryParams} from './feature_flag_serializer';

const COLOR_GROUP_REGEX_VALUE_PREFIX = 'regex:';
Expand All @@ -49,17 +50,20 @@ const COLOR_GROUP_REGEX_BY_EXP_VALUE_PREFIX = 'regex_by_exp:';
/**
* Provides deeplinking for the core dashboards page.
*
* Note: Pinned cards are NO LONGER stored in the URL to avoid URL length
* limitations. Pins are now stored in localStorage via the profile system.
* Note: Pinned cards and smoothing are NO LONGER stored in the URL.
* Pins are stored in localStorage via the profile system to avoid URL length
* limitations. Smoothing is persisted in localStorage only.
* See https://github.com/tensorflow/tensorboard/issues/4242 for context.
*
* The URL still supports:
* - tagFilter: for filtering displayed tags
* - smoothing: scalar smoothing setting
* - runColorGroup: run color grouping configuration
* - runFilter: run selector regex filter
* - Feature flags
*
* For backwards compatibility, smoothing and pinnedCards are still
* deserialized from the URL if present (e.g. from old bookmarked URLs).
*
* For sharing complete dashboard configurations including pins, use the
* profile export/import functionality.
*/
Expand Down Expand Up @@ -91,19 +95,6 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
);
})
),
store.select(selectors.getMetricsSettingOverrides).pipe(
map((settingOverrides) => {
if (Number.isFinite(settingOverrides.scalarSmoothing)) {
return [
{
key: SMOOTHING_KEY,
value: String(settingOverrides.scalarSmoothing),
},
];
}
return [];
})
),
store.select(selectors.getRunUserSetGroupBy).pipe(
map((groupBy) => {
if (!groupBy) return [];
Expand Down
39 changes: 12 additions & 27 deletions tensorbored/webapp/routes/dashboard_deeplink_provider_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,30 +69,19 @@ describe('core deeplink provider', () => {

describe('time series', () => {
describe('smoothing state', () => {
it('serializes the smoothing state to the URL', () => {
it('does NOT serialize smoothing to the URL (stored in localStorage instead)', () => {
store.overrideSelector(selectors.getMetricsSettingOverrides, {
scalarSmoothing: 0,
scalarSmoothing: 0.6,
});
store.overrideSelector(selectors.getMetricsTagFilter, 'trigger');
store.refreshState();

expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[
{
key: 'smoothing',
value: '0',
},
]
);
});

it('does not reflect state when there is no override', () => {
store.overrideSelector(selectors.getMetricsSettingOverrides, {});

store.refreshState();

expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
[]
);
const lastEmission =
queryParamsSerialized[queryParamsSerialized.length - 1];
expect(lastEmission).toBeDefined();
expect(
lastEmission.find((p: {key: string}) => p.key === 'smoothing')
).toBeUndefined();
});

it('deserializes the state in the URL without much sanitization', () => {
Expand Down Expand Up @@ -176,23 +165,19 @@ describe('core deeplink provider', () => {
it('serializes nothing when states are empty', () => {
store.overrideSelector(selectors.getPinnedCardsWithMetadata, []);
store.overrideSelector(selectors.getUnresolvedImportedPinnedCards, []);
// Trigger an emission by changing a serialized selector (smoothing)
store.overrideSelector(selectors.getMetricsSettingOverrides, {
scalarSmoothing: 0.5,
});
// Trigger an emission by changing a serialized selector (tag filter)
store.overrideSelector(selectors.getMetricsTagFilter, 'trigger');
store.refreshState();

// Verify emission occurred and does NOT contain pinned cards
const lastEmission =
queryParamsSerialized[queryParamsSerialized.length - 1];
expect(lastEmission).toBeDefined();
// Should only have smoothing, no pinnedCards
expect(
lastEmission.find((p: {key: string}) => p.key === 'pinnedCards')
).toBeUndefined();
expect(
lastEmission.find((p: {key: string}) => p.key === 'smoothing')
).toBeDefined();
).toBeUndefined();
});

// Deserialization still works for backwards compatibility with old URLs
Expand Down