Page MenuHomePhabricator

Allow PresetDefinitions to wrap ModeDefinitions
AbandonedPublic

Authored by sandyryza on May 22 2020, 9:59 PM.

Details

Summary

I'm not convinced that it's the right approach, but I figured it would at least spur discussion.

Defining a prod preset requires writing "prod" five times. I have found this frustrating.

prod_mode = ModeDefinition(name='prod', resource_defs={'pyspark': pyspark_resource})

prod_preset = PresetDefinition.from_pkg_resources(
    name='prod',
    mode='prod',
    pkg_resource_defs=[('package', 'file.yaml')],
)

With this change, it takes two times:

prod_preset = PresetDefinition.from_pkg_resources(
    mode=ModeDefinition(name='prod', resource_defs={'pyspark': pyspark_resource}),
    pkg_resource_defs=[('package', 'file.yaml')],
)

This change is backwards compatible, but the way the preset gets its name is a little bit intricate/magical.

Test Plan

bk

Diff Detail

Repository
R1 dagster
Branch
preset_wrap_mode (branched from master)
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

sandyryza created this revision.May 22 2020, 9:59 PM
Harbormaster returned this revision to the author for changes because remote builds failed.May 22 2020, 10:10 PM
Harbormaster failed remote builds in B11871: Diff 14605!
sandyryza retitled this revision from Allow PresetDefinitions to wrap ModeDefinitionsi to Allow PresetDefinitions to wrap ModeDefinitions.May 22 2020, 10:18 PM
sandyryza edited the summary of this revision. (Show Details)
sandyryza added reviewers: schrockn, alangenfeld.
sandyryza edited the summary of this revision. (Show Details)
sandyryza updated this revision to Diff 14918.May 27 2020, 6:53 PM

get some tests passing

Harbormaster returned this revision to the author for changes because remote builds failed.May 27 2020, 7:06 PM
Harbormaster failed remote builds in B12144: Diff 14918!
sandyryza edited the summary of this revision. (Show Details)May 27 2020, 7:51 PM
sandyryza updated this revision to Diff 14922.May 27 2020, 8:01 PM

fix tests

sandyryza requested review of this revision.May 27 2020, 8:16 PM
sandyryza edited the summary of this revision. (Show Details)May 27 2020, 8:17 PM

hmm definitely not crazy - will need to think on this one a bit

schrockn requested changes to this revision.May 28 2020, 7:27 PM

Per call today, we are going to try an approach where we do something akin to ConfigMapping, but on the resource granularity. This will be an easy facility to "curry" config into a mode

This revision now requires changes to proceed.May 28 2020, 7:27 PM