Page MenuHomePhabricator

Set metadata on solid instances
ClosedPublic

Authored by alangenfeld on Dec 20 2019, 9:50 PM.

Details

Summary

depends on D1919

Test Plan

Unit

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

max created this revision.Dec 20 2019, 9:50 PM
alangenfeld added inline comments.Dec 20 2019, 10:26 PM
python_modules/dagster/dagster/core/definitions/dependency.py
152–153

is this the right order?

python_modules/dagster/dagster/utils/__init__.py
169–170

whats up with this? why override the dict hash for a frozen dict?

schrockn added inline comments.Dec 20 2019, 10:31 PM
python_modules/dagster/dagster/utils/__init__.py
170

yeah don't do this. dicts are not hashable for a reason

max added inline comments.Dec 21 2019, 4:36 AM
python_modules/dagster/dagster/core/definitions/dependency.py
152–153

yes, stupidly

python_modules/dagster/dagster/utils/__init__.py
170

ok, that's fine -- we can work around by using a different representation for the metadata

schrockn requested changes to this revision.Dec 21 2019, 3:16 PM
schrockn added inline comments.
python_modules/dagster/dagster/core/definitions/dependency.py
152–153

should we change the name of dict_merge so its easier intuit what the precedence semantics are?

This revision now requires changes to proceed.Dec 21 2019, 3:16 PM
alangenfeld commandeered this revision.Jan 27 2020, 11:51 PM
alangenfeld planned changes to this revision.
alangenfeld edited reviewers, added: max; removed: alangenfeld.

I'll update this to handle resource_mapper_fn as well

alangenfeld updated this revision to Diff 9015.Jan 28 2020, 5:13 PM

add tags type

alangenfeld planned changes to this revision.Jan 28 2020, 5:15 PM
schrockn accepted this revision.Jan 30 2020, 12:03 AM

excellent

python_modules/dagster/dagster/core/definitions/composition.py
271

generally think including key_type= and value_type= improves readability but nbd

python_modules/dagster/dagster/core/definitions/container.py
228

not one-liner below because of whitespace/wrapping issues presumably

python_modules/dagster/dagster_tests/core_tests/definitions_tests/test_composition.py
574

cool

This revision is now accepted and ready to land.Jan 30 2020, 12:03 AM
This revision was automatically updated to reflect the committed changes.