Page MenuHomePhabricator

Set metadata on solid instances
Needs RevisionPublic

Authored by max on Fri, Dec 20, 9:50 PM.

Details

Test Plan

Unit

Diff Detail

Repository
R1 dagster
Branch
solid-instance-tags
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

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

is this the right order?

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

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

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

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

max added inline comments.Sat, Dec 21, 4:36 AM
python_modules/dagster/dagster/core/definitions/dependency.py
163–164

yes, stupidly

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

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

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

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

This revision now requires changes to proceed.Sat, Dec 21, 3:16 PM