Page MenuHomeElementl
Differential D9141

Add custom serializer to config type snapshot
ClosedPublic
Actions

Authored by • dgibson on Jul 29 2021, 2:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 10, 11:53 PM
Unknown Object (File)
Fri, Mar 10, 11:53 PM
Unknown Object (File)
Fri, Mar 10, 11:53 PM
Unknown Object (File)
Fri, Mar 10, 11:53 PM
Unknown Object (File)
Fri, Mar 10, 11:49 PM
Unknown Object (File)
Fri, Mar 10, 11:49 PM
Unknown Object (File)
Thu, Mar 9, 5:22 PM
Unknown Object (File)
Thu, Mar 9, 2:28 PM
View All 99 Files
Subscribers
None

Details

Reviewers
• cdecarolis
alangenfeld
Commits
R1:d6777854326a: Add custom serializer to config type snapshot
Summary

This makes the pipeline snapshot Id stable if you haven't made any changes to the run config.

Test Plan

BK, verify pipeline snapshot ID changes in https://dagster.phacility.com/D8550 no longer needed

Diff Detail

Repository
R1 dagster
Branch
stability (branched from master)
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

• dgibson created this revision.Jul 29 2021, 2:06 PM
• dgibson updated this revision to Diff 42569.Jul 29 2021, 2:11 PM

up

Harbormaster failed remote builds in B34439: Diff 42568!Jul 29 2021, 2:30 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 29 2021, 2:35 PM
Harbormaster failed remote builds in B34440: Diff 42569!
alangenfeld added inline comments.Jul 29 2021, 2:40 PM
python_modules/dagster/dagster/serdes/serdes.py
138–173

empty set?

would be nice to add a serdes test case for this as well

169–170

nit: I really had to stare at this for a bit to convince my self this logic condition was correct, might be more legible if verbose or taken out of an inline dict creation

• dgibson updated this revision to Diff 42575.Jul 29 2021, 2:48 PM

mypy

• dgibson published this revision for review.Jul 29 2021, 2:48 PM
alangenfeld accepted this revision.Jul 29 2021, 2:52 PM

^ above comments worth considering

This revision is now accepted and ready to land.Jul 29 2021, 2:52 PM
• dgibson updated this revision to Diff 42577.Jul 29 2021, 3:00 PM

add test case, make logic clearer

This revision was landed with ongoing or failed builds.Jul 29 2021, 3:25 PM
Closed by commit R1:d6777854326a: Add custom serializer to config type snapshot (authored by • dgibson). · Explain Why
This revision was automatically updated to reflect the committed changes.
• dgibson added a commit: R1:d6777854326a: Add custom serializer to config type snapshot.
Harbormaster completed remote builds in B34447: Diff 42577.Jul 29 2021, 3:27 PM
Harbormaster failed remote builds in B34445: Diff 42575!Jul 29 2021, 3:53 PM

Revision Contents
Changeset List

PathSize
python_modules/
dagster-graphql/
dagster_graphql_tests/
graphql/
snapshots/
dagster/
dagster/
config/
14 lines
serdes/
17 lines
dagster_tests/
core_tests/
snap_tests/
snapshots/
general_tests/
13 lines

Diff 42577

View Options

python_modules/dagster-graphql/dagster_graphql_tests/graphql/snapshots/snap_test_all_snapshot_ids.py

Loading...
View Options

python_modules/dagster-graphql/dagster_graphql_tests/graphql/snapshots/snap_test_pipeline_snapshot.py

Loading...
View Options

python_modules/dagster/dagster/config/snap.py

Loading...
View Options

python_modules/dagster/dagster/serdes/serdes.py

Loading...
View Options

python_modules/dagster/dagster_tests/core_tests/snap_tests/snapshots/snap_test_active_data.py

Loading...
View Options

python_modules/dagster/dagster_tests/core_tests/snap_tests/snapshots/snap_test_config_schema_snapshot.py

Loading...
View Options

python_modules/dagster/dagster_tests/core_tests/snap_tests/snapshots/snap_test_execution_plan.py

Loading...
View Options

python_modules/dagster/dagster_tests/core_tests/snap_tests/snapshots/snap_test_pipeline_snap.py

Loading...
View Options

python_modules/dagster/dagster_tests/general_tests/test_serdes.py

Loading...