Skip to content

Commit

Permalink
[4/4] Add asset selection input for new syntax (dagster-io#26000)
Browse files Browse the repository at this point in the history
## Summary & Motivation

Adds an asset selection input that supports the new syntax and has
auto-complete suggestions.

For now this is behind a hidden feature flag.

error state:
<img width="990" alt="Screenshot 2024-11-19 at 1 49 11 AM"
src="https://github.com/user-attachments/assets/e31986bd-f914-463f-a2d7-af590c86cbe5">

key/value suggestions:
<img width="736" alt="Screenshot 2024-11-19 at 1 48 54 AM"
src="https://github.com/user-attachments/assets/fb9cdd96-c4fd-4797-9e6c-2bcbc4d11984">

base suggestions:
<img width="490" alt="Screenshot 2024-11-19 at 1 48 47 AM"
src="https://github.com/user-attachments/assets/452a666a-fefe-46c8-8914-4db3aa483aaf">




## How I Tested These Changes

Manual testing for now until i figure out a good way to test
CodeMirror...

To enable the feature flag:
```
localStorage.setItem("DAGSTER_FLAGS", JSON.stringify({
    ...JSON.parse(localStorage.DAGSTER_FLAGS),
    'flagAssetSelectionSyntax': true
}))
```


https://github.com/user-attachments/assets/d158f5e0-527c-4350-ac79-7c7518a11c60
  • Loading branch information
salazarm authored and pskinnerthyme committed Dec 16, 2024
1 parent d303ccc commit 31bc8d9
Show file tree
Hide file tree
Showing 11 changed files with 709 additions and 51 deletions.
19 changes: 0 additions & 19 deletions js_modules/dagster-ui/packages/app-oss/src/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,3 @@ export default function IndexPage() {
</div>
);
}

/**
* Ignore hard navigation error (only happens in dev mode).
*/
if (process.env.NODE_ENV === 'development' && typeof window !== 'undefined') {
const originalError = window.Error;
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
window.Error = function Error(...args) {
if (args[0]?.includes('Invariant: attempted to hard navigate to the same URL')) {
return;
}
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const err = originalError(...args);
Object.setPrototypeOf(err, window.Error.prototype);
return err;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ import uniq from 'lodash/uniq';
import without from 'lodash/without';
import * as React from 'react';
import {useCallback, useLayoutEffect, useMemo, useState} from 'react';
import {FeatureFlag} from 'shared/app/FeatureFlags.oss';
import {AssetGraphAssetSelectionInput} from 'shared/asset-graph/AssetGraphAssetSelectionInput.oss';
import {useAssetGraphExplorerFilters} from 'shared/asset-graph/useAssetGraphExplorerFilters.oss';
import {AssetSelectionInput} from 'shared/asset-selection/input/AssetSelectionInput.oss';
import styled from 'styled-components';

import {AssetEdges} from './AssetEdges';
Expand Down Expand Up @@ -50,6 +52,7 @@ import {
useFullAssetGraphData,
} from './useAssetGraphData';
import {AssetLocation, useFindAssetLocation} from './useFindAssetLocation';
import {featureEnabled} from '../app/Flags';
import {AssetLiveDataRefreshButton} from '../asset-data/AssetLiveDataProvider';
import {LaunchAssetExecutionButton} from '../assets/LaunchAssetExecutionButton';
import {AssetKey} from '../assets/types';
Expand Down Expand Up @@ -733,13 +736,21 @@ const AssetGraphExplorerWithData = ({
)}
<div>{filterButton}</div>
<GraphQueryInputFlexWrap>
<AssetGraphAssetSelectionInput
items={graphQueryItems}
value={explorerPath.opsQuery}
placeholder="Type an asset subset…"
onChange={onChangeAssetSelection}
popoverPosition="bottom-left"
/>
{featureEnabled(FeatureFlag.flagAssetSelectionSyntax) ? (
<AssetSelectionInput
assets={graphQueryItems}
value={explorerPath.opsQuery}
onChange={onChangeAssetSelection}
/>
) : (
<AssetGraphAssetSelectionInput
items={graphQueryItems}
value={explorerPath.opsQuery}
placeholder="Type an asset subset…"
onChange={onChangeAssetSelection}
popoverPosition="bottom-left"
/>
)}
</GraphQueryInputFlexWrap>
<AssetLiveDataRefreshButton />
<LaunchAssetExecutionButton
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,246 @@
import CodeMirror, {HintFunction, Hints} from 'codemirror';

import {assertUnreachable} from '../../app/Util';
import {AssetGraphQueryItem} from '../../asset-graph/useAssetGraphData';
import {buildRepoPathForHuman} from '../../workspace/buildRepoAddress';

export const possibleKeywords = [
'key:',
'key_substring:',
'tag:',
'owner:',
'group:',
'kind:',
'code_location:',
'sinks()',
'roots()',
'not',
'*',
'+',
];

const logicalOperators = ['and', 'or', '*', '+'];

export const createAssetSelectionHint = (assets: AssetGraphQueryItem[]): HintFunction => {
const assetNamesSet: Set<string> = new Set();
const tagNamesSet: Set<string> = new Set();
const ownersSet: Set<string> = new Set();
const groupsSet: Set<string> = new Set();
const kindsSet: Set<string> = new Set();
const codeLocationSet: Set<string> = new Set();

assets.forEach((asset) => {
assetNamesSet.add(asset.name);
asset.node.tags.forEach((tag) => {
if (tag.key && tag.value) {
tagNamesSet.add(`${tag.key}=${tag.value}`);
} else {
tagNamesSet.add(tag.key);
}
});
asset.node.owners.forEach((owner) => {
switch (owner.__typename) {
case 'TeamAssetOwner':
ownersSet.add(owner.team);
break;
case 'UserAssetOwner':
ownersSet.add(owner.email);
break;
default:
assertUnreachable(owner);
}
});
if (asset.node.groupName) {
groupsSet.add(asset.node.groupName);
}
asset.node.kinds.forEach((kind) => {
kindsSet.add(kind);
});
const location = buildRepoPathForHuman(
asset.node.repository.name,
asset.node.repository.location.name,
);
codeLocationSet.add(location);
});

const assetNames = Array.from(assetNamesSet).map(addQuotesToString);
const tagNames = Array.from(tagNamesSet).map(addQuotesToString);
const owners = Array.from(ownersSet).map(addQuotesToString);
const groups = Array.from(groupsSet).map(addQuotesToString);
const kinds = Array.from(kindsSet).map(addQuotesToString);
const codeLocations = Array.from(codeLocationSet).map(addQuotesToString);

return (cm: CodeMirror.Editor): Hints | undefined => {
const cursor = cm.getCursor();
const token = cm.getTokenAt(cursor);

const indexOfToken: number = token.state.tokenIndex - 1;
const allTokens = token.state.tokens;

const previous2Tokens =
token.string.trim() === ''
? [allTokens[indexOfToken - 1]?.text, allTokens[indexOfToken]?.text]
: [allTokens[indexOfToken - 2]?.text, allTokens[indexOfToken - 1]?.text];

let start = token.start;
const end = token.end;
const tokenString = token.string.trim();
const tokenUpToCursor = cm.getRange(CodeMirror.Pos(cursor.line, start), cursor).trim();
const unquotedTokenString = removeQuotesFromString(tokenString);

const isAfterAttributeValue = previous2Tokens[0] === ':' && previous2Tokens[1] !== undefined;

const isAfterParenthesizedExpressions =
// if tokenUpToCursor === '' and tokenString ===') then the cursor is to the left of the parenthesis
previous2Tokens[1] === ')' || (tokenString === ')' && tokenUpToCursor !== '');

const isInKeyValue =
(previous2Tokens[1] === ':' && token.string.trim() !== '') || tokenString === ':';

const isTraversal = /^[*+]+$/.test(tokenString);

const tokensBefore = allTokens
.slice(0, indexOfToken + 1)
.map((token: any) => token.text?.trim());
const preTraversal = isTraversal && isPreTraversal(tokensBefore);
const isPostTraversal = isTraversal && !preTraversal;

const isEndOfKeyValueExpression =
isInKeyValue &&
tokenString.endsWith('"') &&
tokenString.length > 2 &&
tokenUpToCursor.endsWith('"');

const isAfterTraversal = ['+', '*'].includes(previous2Tokens[1]);

function getSuggestions() {
if (isEndOfKeyValueExpression) {
start = end;
}

if (
isPostTraversal ||
isAfterAttributeValue ||
isAfterParenthesizedExpressions ||
isEndOfKeyValueExpression ||
isAfterTraversal
) {
return logicalOperators;
}

if (isInKeyValue) {
let type = previous2Tokens[0];
if (tokenString === ':') {
type = previous2Tokens[1];
}
switch (type) {
case 'key_substring':
case 'key':
return assetNames;
case 'tag':
return tagNames;
case 'owner':
return owners;
case 'group':
return groups;
case 'kind':
return kinds;
case 'code_location':
return codeLocations;
}
}

if (tokenString === '' || tokenString === '(' || tokenString === ')' || preTraversal) {
return possibleKeywords;
}
return [
`key_substring:"${unquotedTokenString}"`,
`key:"${unquotedTokenString}"`,
...possibleKeywords,
];
}

let suggestions = getSuggestions();

if (!(isTraversal || isEndOfKeyValueExpression || ['', ':', '(', ')'].includes(tokenString))) {
suggestions = suggestions.filter(
(item) =>
item.startsWith(tokenString) ||
item.startsWith(unquotedTokenString) ||
item.includes(`:"${unquotedTokenString}`) ||
item.startsWith(`"${unquotedTokenString}`),
);
}

const list = suggestions.map((item) => {
let text = item;
if (token.string[0] === ' ') {
text = ' ' + item;
}
if (tokenString === ':') {
text = `:${item}`;
}

if (tokenString === '(') {
text = `(${text}`;
}
if (tokenString === ')') {
if (isAfterParenthesizedExpressions) {
text = `) ${text}`;
} else {
text = `${text})`;
}
}

const trimmedText = text.trim();

if (isTraversal) {
if (text === '+' || text === '*') {
text = `${tokenString}${text}`;
} else if (trimmedText === 'and' || trimmedText === 'or' || trimmedText === 'not') {
text = `${tokenString} ${text}`;
}
} else if (trimmedText === 'and' || trimmedText === 'or' || trimmedText === 'not') {
text = ` ${trimmedText} `; // Insert spaces around the logical operator
}

return {
text: text.replaceAll(/(\s)+/g, ' ').replaceAll(/(")+/g, '"'),
displayText: removeQuotesFromString(item),
};
});

return {
list,
from: CodeMirror.Pos(cursor.line, start),
to: CodeMirror.Pos(cursor.line, end),
};
};
};

const removeQuotesFromString = (value: string) => {
if (value.length > 1 && value[0] === '"' && value[value.length - 1] === '"') {
return value.slice(1, value.length - 1);
}
return value;
};

const addQuotesToString = (value: string) => `"${value}"`;

const isPreTraversal = (tokensBefore: string[]) => {
// If there are no tokens before, it's the start of the line
if (tokensBefore.length === 0) {
return true;
}

const previousToken = tokensBefore[tokensBefore.length - 1];

// Check if the previous token is 'and', 'or', or '('
return (
previousToken === 'and' ||
previousToken === 'or' ||
previousToken === '(' ||
!previousToken ||
tokensBefore.every((token) => ['*', '+'].includes(token))
);
};
Loading

0 comments on commit 31bc8d9

Please sign in to comment.