Page MenuHomePhabricator

RFC: decorator-based mode definition
Needs ReviewPublic

Authored by schrockn on Oct 19 2020, 4:32 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Perusing both our test cases as well as during user sessions, noticing that modes
always end up being global variables, with the name encoded in a couple places (mode name
and variable name specifically) and generally a bit messy.

The decorator-based approach seems cleaner. Additionaly I believe we are on the path
to eliminating many of the non-resource arguments to mode, so the default case
should just the resources returned by the function.

Test Plan

BK

Diff Detail

Repository
R1 dagster
Branch
new-mode-api
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 19 2020, 4:54 PM
Harbormaster failed remote builds in B19762: Diff 23983!

This is also an elegant way to change the arguments to mode definition. For example I think we could singularize executor_defs, not support system_storage_defs, move logger_defs to a special resource key, etc.

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 19 2020, 6:22 PM
Harbormaster failed remote builds in B19773: Diff 23994!

I think this is a net positive. My two reservations would be:

  • Two ways of doing something is worse than one.
  • Operating on a function instead of a resource dict gives users an extra degree of flexibility that they might find confusing.

I don't think in this case two ways of doing things is worse. It's properly layered. Would you eliminate @solid?

Would you eliminate @solid?

I think there are almost no cases where SolidDefinition is preferable to @solid, because solids wrap functions. Unless we strongly guide them otherwise, I think some people might prefer to use ModeDefinition(resource_defs), because they might find putting their resource_defs inside a function weird.

Using SolidDefinition is desirable for framework authors. This is a layered system