Page MenuHomePhabricator

Make anonymous types in config type system have stable keys
ClosedPublic

Authored by schrockn on Nov 20 2019, 3:32 PM.

Details

Summary

This has been a long-standing technical liability that IMO
become intolerable with the upcoming work to persist the type system
metadata in a database. Instead of just assigning an incremental
counter to Dicts, Selectors, etc to assign key names (so that we
can do things like have the config editor work) instead this
assigns the key based on the fields passed into types.

Critically this makes it so that import order will no longer alter
the keys.

Test Plan

BK and run dagit.

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

schrockn retitled this revision from make dictionary keys stable to Make anonymous types in config type system have stable keys.
schrockn edited the summary of this revision. (Show Details)
schrockn edited the test plan for this revision. (Show Details)

upmessage

test field order does not matter

test for recursive /deeply nested dict structure

This revision now requires changes to proceed.Nov 20 2019, 5:37 PM

agree with alex we should be rigorous about test coverage but this is such a good thing to finally put to bed. resigining in favor of @alangenfeld ๐Ÿ‘

python_modules/dagster/dagster_tests/core_tests/types_tests/test_field_hash.py
38

specifically, i'd like to make sure *this* is still true for deeply nested structures

This revision is now accepted and ready to land.Nov 20 2019, 11:13 PM