Skip to content

Commit

Permalink
[dagit] Avoid copying definition-time tags into launchpad state (#6851)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bengotow
bengotow committed Mar 2, 2022
1 parent f0323af commit de1b5ef
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 91 deletions.
130 changes: 64 additions & 66 deletions js_modules/dagit/packages/core/src/launchpad/LaunchpadSessionContainer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
SplitPanelContainer,
} from '@dagster-io/ui';
import merge from 'deepmerge';
import {uniqBy} from 'lodash';
import * as React from 'react';
import styled from 'styled-components/macro';
import * as yaml from 'yaml';
Expand Down Expand Up @@ -170,6 +171,7 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
);

const currentSession = data.sessions[data.current];
const tagsFromSession = React.useMemo(() => currentSession.tags || [], [currentSession]);

const pipelineSelector = {
...repoAddressToSelector(repoAddress),
Expand Down Expand Up @@ -224,25 +226,6 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
};

const onRemoveExtraPaths = (paths: string[]) => {
const deletePropertyPath = (obj: any, path: string) => {
const parts = path.split('.');

// Here we iterate through the parts of the path to get to
// the second to last nested object. This is so we can call `delete` using
// this object and the last part of the path.
for (let i = 0; i < parts.length - 1; i++) {
obj = obj[parts[i]];
if (typeof obj === 'undefined') {
return;
}
}

const lastKey = parts.pop();
if (lastKey) {
delete obj[lastKey];
}
};

let runConfigData = {};
try {
// Note: parsing `` returns null rather than an empty object,
Expand All @@ -252,9 +235,7 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
for (const path of paths) {
deletePropertyPath(runConfigData, path);
}

const runConfigYaml = yaml.stringify(runConfigData);
onSaveSession({runConfigYaml});
onSaveSession({runConfigYaml: yaml.stringify(runConfigData)});
} catch (err) {
showCustomAlert({title: 'Invalid YAML', body: YAML_SYNTAX_INVALID});
return;
Expand Down Expand Up @@ -286,7 +267,6 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
return;
}

const tags = currentSession.tags || [];
let runConfigData = {};
try {
// Note: parsing `` returns null rather than an empty object,
Expand All @@ -303,27 +283,35 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
selector: pipelineSelector,
mode: currentSession.mode || 'default',
executionMetadata: {
tags: [
...tags.map((tag) => ({key: tag.key, value: tag.value})),
// pass solid selection query via tags
// clean up https://github.com/dagster-io/dagster/issues/2495
...(currentSession.solidSelectionQuery
? [
{
key: DagsterTag.SolidSelection,
value: currentSession.solidSelectionQuery,
},
]
: []),
...(currentSession?.base?.['presetName']
? [
{
key: DagsterTag.PresetName,
value: currentSession?.base?.['presetName'],
},
]
: []),
],
tags: uniqBy(
[
// pass solid selection query via tags
// clean up https://github.com/dagster-io/dagster/issues/2495
...(currentSession.solidSelectionQuery
? [
{
key: DagsterTag.SolidSelection,
value: currentSession.solidSelectionQuery,
},
]
: []),
...(currentSession?.base?.['presetName']
? [
{
key: DagsterTag.PresetName,
value: currentSession?.base?.['presetName'],
},
]
: []),

...tagsFromSession.map(onlyKeyAndValue),

// note, we apply these last - uniqBy uses the first value it sees for
// each key, so these can be overridden in the session
...pipeline.tags.map(onlyKeyAndValue),
],
(tag) => tag.key,
),
},
},
};
Expand Down Expand Up @@ -375,22 +363,14 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
};

const onSelectPreset = async (preset: Preset) => {
const tagsDict: {[key: string]: string} = [...(pipeline?.tags || []), ...preset.tags].reduce(
(tags, kv) => {
tags[kv.key] = kv.value;
return tags;
},
{},
);

onSaveSession({
base: {presetName: preset.name},
name: preset.name,
runConfigYaml: preset.runConfigYaml || '',
solidSelection: preset.solidSelection,
solidSelectionQuery: preset.solidSelection === null ? '*' : preset.solidSelection.join(','),
mode: preset.mode,
tags: Object.entries(tagsDict).map(([key, value]) => ({key, value})),
tags: preset.tags.map(onlyKeyAndValue),
needsRefresh: false,
});
};
Expand Down Expand Up @@ -424,14 +404,13 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro

const {partition} = data.partitionSetOrError;

let tags;
let tags: {key: string; value: string}[] = [];
if (partition.tagsOrError.__typename === 'PythonError') {
tags = (pipeline?.tags || []).slice();
showCustomAlert({
body: <PythonErrorInfo error={partition.tagsOrError} />,
});
} else {
tags = [...(pipeline?.tags || []), ...partition.tagsOrError.results];
tags = [...partition.tagsOrError.results];
}

let runConfigYaml;
Expand Down Expand Up @@ -518,13 +497,6 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
tagEditorOpen,
} = state;

const tagsFromDefinition: PipelineRunTag[] = React.useMemo(() => pipeline.tags || [], [pipeline]);
const tagsFromSession = React.useMemo(() => currentSession.tags || [], [currentSession]);
const allTags = React.useMemo(
() => ({fromDefinition: tagsFromDefinition, fromSession: tagsFromSession}),
[tagsFromDefinition, tagsFromSession],
);

const refreshableSessionBase = React.useMemo(() => {
const {base, needsRefresh} = currentSession;
if (
Expand Down Expand Up @@ -587,7 +559,7 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
</>
)}
<TagEditor
tagsFromDefinition={tagsFromDefinition}
tagsFromDefinition={pipeline.tags}
tagsFromSession={tagsFromSession}
onChange={saveTags}
open={tagEditorOpen}
Expand Down Expand Up @@ -617,12 +589,16 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
<SessionSettingsSpacer />
<SecondPanelToggle axis="horizontal" container={editorSplitPanelContainer} />
</SessionSettingsBar>
{allTags.fromDefinition.length || allTags.fromSession.length ? (
{pipeline.tags.length || tagsFromSession.length ? (
<Box
padding={{vertical: 8, left: 12, right: 0}}
border={{side: 'bottom', width: 1, color: ColorsWIP.Gray200}}
>
<TagContainer tags={allTags} onRequestEdit={openTagEditor} />
<TagContainer
tagsFromDefinition={pipeline.tags}
tagsFromSession={tagsFromSession}
onRequestEdit={openTagEditor}
/>
</Box>
) : null}
{refreshableSessionBase ? (
Expand Down Expand Up @@ -711,6 +687,28 @@ const LaunchpadSessionContainer: React.FC<LaunchpadSessionContainerProps> = (pro
// eslint-disable-next-line import/no-default-export
export default LaunchpadSessionContainer;

// This helper removes __typename, which prevents tags from being passed back to GraphQL
const onlyKeyAndValue = ({key, value}: {key: string; value: string}) => ({key, value});

const deletePropertyPath = (obj: any, path: string) => {
const parts = path.split('.');

// Here we iterate through the parts of the path to get to
// the second to last nested object. This is so we can call `delete` using
// this object and the last part of the path.
for (let i = 0; i < parts.length - 1; i++) {
obj = obj[parts[i]];
if (typeof obj === 'undefined') {
return;
}
}

const lastKey = parts.pop();
if (lastKey) {
delete obj[lastKey];
}
};

const PREVIEW_CONFIG_QUERY = gql`
query PreviewConfigQuery(
$pipeline: PipelineSelector!
Expand Down
53 changes: 32 additions & 21 deletions js_modules/dagit/packages/core/src/launchpad/TagEditor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@ interface ITagEditorProps {
}

interface ITagContainerProps {
tags: {
fromDefinition?: PipelineRunTag[];
fromSession?: PipelineRunTag[];
};
tagsFromDefinition?: PipelineRunTag[];
tagsFromSession: PipelineRunTag[];
onRequestEdit: () => void;
}

Expand All @@ -44,6 +42,14 @@ export const TagEditor: React.FC<ITagEditorProps> = ({
tagsFromSession.length ? tagsFromSession : [{key: '', value: ''}],
);

// Reset the edit state when you close and re-open the modal, or when
// tagsFromSession change while the modal is closed.
React.useEffect(() => {
if (!open) {
setEditState(tagsFromSession.length ? tagsFromSession : [{key: '', value: ''}]);
}
}, [tagsFromSession, open]);

const toSave: PipelineRunTag[] = editState
.map((tag: PipelineRunTag) => ({
key: tag.key.trim(),
Expand Down Expand Up @@ -176,26 +182,31 @@ export const TagEditor: React.FC<ITagEditorProps> = ({
);
};

export const TagContainer = ({tags, onRequestEdit}: ITagContainerProps) => {
const {fromDefinition = [], fromSession = []} = tags;
export const TagContainer = ({
tagsFromSession,
tagsFromDefinition,
onRequestEdit,
}: ITagContainerProps) => {
return (
<Container>
<TagList>
{fromDefinition.map((tag, idx) => {
const {key} = tag;
const anyOverride = fromSession.some((sessionTag) => sessionTag.key === key);
if (anyOverride) {
return (
<Tooltip key={key} content="Overriden by custom tag value" placement="top">
<span style={{opacity: 0.2}}>
<RunTag tag={tag} key={idx} />
</span>
</Tooltip>
);
}
return <RunTag tag={tag} key={idx} />;
})}
{fromSession.map((tag, idx) => (
{tagsFromDefinition
? tagsFromDefinition.map((tag, idx) => {
const {key} = tag;
const anyOverride = tagsFromSession.some((sessionTag) => sessionTag.key === key);
if (anyOverride) {
return (
<Tooltip key={key} content="Overriden by custom tag value" placement="top">
<span style={{opacity: 0.2}}>
<RunTag tag={tag} key={idx} />
</span>
</Tooltip>
);
}
return <RunTag tag={tag} key={idx} />;
})
: undefined}
{tagsFromSession.map((tag, idx) => (
<RunTag tag={tag} key={idx} />
))}
</TagList>
Expand Down
5 changes: 1 addition & 4 deletions js_modules/dagit/packages/core/src/partitions/PartitionsBackfill.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,10 +407,7 @@ export const PartitionsBackfillPartitionSelector: React.FC<{
<strong>Tags</strong>
{tags.length ? (
<div style={{border: `1px solid ${ColorsWIP.Gray300}`, borderRadius: 8, padding: 3}}>
<TagContainer
tags={{fromSession: tags}}
onRequestEdit={() => setTagEditorOpen(true)}
/>
<TagContainer tagsFromSession={tags} onRequestEdit={() => setTagEditorOpen(true)} />
</div>
) : (
<div>
Expand Down

0 comments on commit de1b5ef

Please sign in to comment.