Skip to content

Commit

Permalink
add partial tag autocomplete for run filter input (#12410)
Browse files Browse the repository at this point in the history
### Summary & Motivation
We disabled tag autocomplete because we were fetching wayyyy too much
data for this feature.

But we were also fetching tags in a really dumb way, fetching all unique
key/value pairs in all run history, and relying on the typeahead to
filter appropriately.

This PR breaks up tag searches into two phases, first fetching the
distinct tag keys, then fixing the tag key and lazily searching for the
unique values given a fixed tag key. These instance storage methods were
added in #12348 and the
graphql endpoints are in
#12409.

The cardinality of the tag key space is assumed to be low (<1000). The
cardinality of the tag value space can be pretty high, especially for
some keys like `dagster/partition`, but we can add additional logic down
the road for doing prefix searches in the lazy tag search, along with a
hard limit.

Tracked in #12104

### How I Tested These Changes


https://user-images.githubusercontent.com/1040172/219518626-cad3be26-f4aa-4d9f-876d-a2f23f264409.mov
  • Loading branch information
prha authored and clairelin135 committed Feb 22, 2023
1 parent 2214ad6 commit f131a97
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 8 deletions.
79 changes: 76 additions & 3 deletions js_modules/dagit/packages/core/src/runs/RunsFilterInput.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {gql, useLazyQuery} from '@apollo/client';
import {
SuggestionProvider,
TokenizingField,
Expand All @@ -12,6 +13,18 @@ import {RunsFilter, RunStatus} from '../graphql/types';
import {useQueryPersistedState} from '../hooks/useQueryPersistedState';
import {DagsterRepoOption, useRepositoryOptions} from '../workspace/WorkspaceContext';

import {
RunTagKeysQuery,
RunTagValuesQuery,
RunTagValuesQueryVariables,
} from './types/RunsFilterInput.types';

type RunTags = Array<{
__typename: 'PipelineTagAndValues';
key: string;
values: Array<string>;
}>;

export type RunFilterTokenType = 'id' | 'status' | 'pipeline' | 'job' | 'snapshotId' | 'tag';

export type RunFilterToken = {
Expand Down Expand Up @@ -107,6 +120,9 @@ export function runsFilterForSearchTokens(search: TokenizingFieldValue[]) {

function searchSuggestionsForRuns(
repositoryOptions: DagsterRepoOption[],
runTagKeys?: string[],
selectedRunTagKey?: string,
runTagValues?: RunTags,
enabledFilters?: RunFilterTokenType[],
): SuggestionProvider[] {
const pipelineNames = new Set<string>();
Expand All @@ -123,7 +139,7 @@ function searchSuggestionsForRuns(
}
}

const suggestions: {token: RunFilterTokenType; values: () => string[]}[] = [
const suggestions: {token: RunFilterTokenType; values: () => string[]; textOnly?: boolean}[] = [
{
token: 'id',
values: () => [],
Expand All @@ -142,7 +158,16 @@ function searchSuggestionsForRuns(
},
{
token: 'tag',
values: () => [],
values: () => {
if (!selectedRunTagKey) {
return (runTagKeys || []).map((key) => `${key}`);
}
return (runTagValues || [])
.filter(({key}) => key === selectedRunTagKey)
.map(({values}) => values.map((value) => `${selectedRunTagKey}=${value}`))
.flat();
},
textOnly: !selectedRunTagKey,
},
{
token: 'snapshotId',
Expand Down Expand Up @@ -171,10 +196,39 @@ export const RunsFilterInput: React.FC<RunsFilterInputProps> = ({
enabledFilters,
}) => {
const {options} = useRepositoryOptions();
const [selectedTagKey, setSelectedTagKey] = React.useState<string | undefined>();
const [fetchTagKeys, {data: tagKeyData}] = useLazyQuery<RunTagKeysQuery>(RUN_TAG_KEYS_QUERY);
const [fetchTagValues, {data: tagValueData}] = useLazyQuery<
RunTagValuesQuery,
RunTagValuesQueryVariables
>(RUN_TAG_VALUES_QUERY, {
variables: {tagKeys: selectedTagKey ? [selectedTagKey] : []},
});

const suggestions = searchSuggestionsForRuns(options, enabledFilters);
React.useEffect(() => {
if (selectedTagKey) {
fetchTagValues();
}
}, [selectedTagKey, fetchTagValues]);

const suggestions = searchSuggestionsForRuns(
options,
tagKeyData?.runTagKeys,
selectedTagKey,
tagValueData?.runTags,
enabledFilters,
);

const search = tokenizedValuesFromStringArray(tokensAsStringArray(tokens), suggestions);
const refreshSuggestions = (text: string) => {
if (!text.startsWith('tag:')) {
return;
}
const tagKeyText = text.slice(4);
if (tagKeyData?.runTagKeys && tagKeyData?.runTagKeys.includes(tagKeyText)) {
setSelectedTagKey(tagKeyText);
}
};

const suggestionProvidersFilter = (
suggestionProviders: SuggestionProvider[],
Expand All @@ -201,13 +255,32 @@ export const RunsFilterInput: React.FC<RunsFilterInputProps> = ({
);
};

const onFocus = React.useCallback(() => fetchTagKeys(), [fetchTagKeys]);

return (
<TokenizingField
values={search}
onChange={(values) => onChange(values as RunFilterToken[])}
onFocus={onFocus}
onTextChange={refreshSuggestions}
suggestionProviders={suggestions}
suggestionProvidersFilter={suggestionProvidersFilter}
loading={loading}
/>
);
};

const RUN_TAG_KEYS_QUERY = gql`
query RunTagKeysQuery {
runTagKeys
}
`;

const RUN_TAG_VALUES_QUERY = gql`
query RunTagValuesQuery($tagKeys: [String!]!) {
runTags(tagKeys: $tagKeys) {
key
values
}
}
`;

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 16 additions & 5 deletions js_modules/dagit/packages/ui/src/components/TokenizingField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface SuggestionProvider {
token?: string;
values: () => string[];
suggestionFilter?: (query: string, suggestion: Suggestion) => boolean;
textOnly?: boolean;
}

export interface Suggestion {
Expand Down Expand Up @@ -47,6 +48,7 @@ interface TokenizingFieldProps {

fullwidth?: boolean;

onTextChange?: (text: string) => void;
suggestionProviders: SuggestionProvider[];
suggestionRenderer?: (suggestion: Suggestion) => React.ReactNode;
suggestionProvidersFilter?: (
Expand Down Expand Up @@ -122,6 +124,7 @@ export const TokenizingField: React.FC<TokenizingFieldProps> = ({
onChange,
onChangeBeforeCommit,
onFocus,
onTextChange,
placeholder,
addOnBlur,
loading,
Expand Down Expand Up @@ -173,7 +176,10 @@ export const TokenizingField: React.FC<TokenizingFieldProps> = ({
return provider
.values()
.filter(suggestionNotUsed)
.map((v) => ({text: provider?.token ? `${provider.token}:${v}` : v, final: true}))
.map((v) => ({
text: provider?.token ? `${provider.token}:${v}` : v,
final: !provider.textOnly,
}))
.filter((s) => suggestionFilter(lastPart, s))
.slice(0, MAX_SUGGESTIONS); // never show too many suggestions for one provider
};
Expand Down Expand Up @@ -214,6 +220,11 @@ export const TokenizingField: React.FC<TokenizingFieldProps> = ({
return suggestionsArr;
}, [atMaxValues, filteredSuggestionProviders, lastPart, parts, typed.length, values]);

const _onTextChange = (text: string) => {
setTyped(text);
onTextChange && onTextChange(text);
};

// We need to manage selection in the dropdown by ourselves. To ensure the
// best behavior we store the active item's index and text (the text allows
// us to relocate it if it's moved and the index allows us to keep selection
Expand Down Expand Up @@ -262,12 +273,12 @@ export const TokenizingField: React.FC<TokenizingFieldProps> = ({
if (suggestion.final) {
// The user has finished a key-value pair
onConfirmText(suggestion.text);
setTyped('');
_onTextChange('');
setActive(null);
setOpen(false);
} else {
// The user has finished a key
setTyped(suggestion.text);
_onTextChange(suggestion.text);
}
};

Expand All @@ -282,7 +293,7 @@ export const TokenizingField: React.FC<TokenizingFieldProps> = ({
return;
}

setTyped('');
_onTextChange('');
onChange([...values, tokenizedValueFromString(str, filteredSuggestionProviders)]);
};

Expand Down Expand Up @@ -395,7 +406,7 @@ export const TokenizingField: React.FC<TokenizingFieldProps> = ({
onChange(next);
}}
onInputChange={(e) => {
setTyped(e.currentTarget.value);
_onTextChange(e.currentTarget.value);

if (onChangeBeforeCommit) {
const tokenized = tokenizedValueFromString(
Expand Down

0 comments on commit f131a97

Please sign in to comment.