Page MenuHomePhabricator

RFC DagsterType.name -> DagsterType.unique_name
ClosedPublic

Authored by alangenfeld on Wed, Nov 11, 11:09 PM.

Details

Summary

There are a bunch of bad error messages since name on DagsterType is this sort of confusing property related to uniqueness, and display_name is the thing you want.

To address this, self.name -> self._name and add a getter for unique_name, and update all the callsites to use either display or unique name accordingly.

This diff doesn't change the input arg to DagsterType, since i think that isn't as problematic and is more costly to change.

Test Plan

bk

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

Harbormaster returned this revision to the author for changes because remote builds failed.Wed, Nov 11, 11:25 PM
Harbormaster failed remote builds in B21021: Diff 25497!
Harbormaster returned this revision to the author for changes because remote builds failed.Thu, Nov 12, 4:15 PM
Harbormaster failed remote builds in B21047: Diff 25530!
python_modules/dagster/dagster/core/types/dagster_type.py
837

This is unrelated to your diff, but we should surface a warning (or even an exception?) here if two types have the same display name

see inline for something to consider but this seems very reasonable

python_modules/dagster/dagster/core/types/dagster_type.py
205–206

should this assert or something if you call it when it's none? I see some places below where its being used as the key in a dict, that would be undesirable if it were null. - maybe a separate has_unique_name method for when you want to check?

This revision is now accepted and ready to land.Wed, Nov 18, 3:37 PM