Page MenuHomeElementl

RFC DagsterType.name -> DagsterType.unique_name
ClosedPublic

Authored by alangenfeld on Nov 11 2020, 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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 11 2020, 11:25 PM
Harbormaster failed remote builds in B21021: Diff 25497!
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 12 2020, 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
206–207

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.Nov 18 2020, 3:37 PM