Page MenuHomePhabricator

Add config mapping context
ClosedPublic

Authored by natekupp on Jun 25 2019, 4:49 PM.

Details

Summary

This adds execution_epoch_time as a tag on the RunConfig object, and threads that through context creation to be added as a property on a ConfigMappingContext object which is provided to config mapping functions.

Test Plan

unit

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

natekupp created this revision.Jun 25 2019, 4:49 PM
natekupp updated this revision to Diff 2253.Jun 25 2019, 6:41 PM

fix dagstermill

natekupp edited the summary of this revision. (Show Details)Jun 25 2019, 6:53 PM
natekupp added reviewers: schrockn, alangenfeld.
schrockn requested changes to this revision.Jun 25 2019, 10:12 PM

Not a huge deal, but I would rather not have to access the parent config through the context. The common case will be *not* access the context in why case you can do

lambda _, cfg: {'some_config': cfg['something_else']}

which i think is nice. thoughts?

This revision now requires changes to proceed.Jun 25 2019, 10:12 PM

cool yeah that sounds reasonable to me, will update

natekupp updated this revision to Diff 2263.Jun 25 2019, 10:34 PM

update per comments

natekupp updated this revision to Diff 2264.Jun 25 2019, 10:36 PM

missed airline demo

schrockn accepted this revision.Jun 25 2019, 10:37 PM
schrockn added inline comments.
python_modules/dagster/dagster/core/definitions/config.py
48

shouldn't this be required?

This revision is now accepted and ready to land.Jun 25 2019, 10:37 PM
natekupp updated this revision to Diff 2265.Jun 25 2019, 10:51 PM

fix and add test

natekupp marked an inline comment as done.Jun 25 2019, 10:53 PM
natekupp added inline comments.
python_modules/dagster/dagster/core/definitions/config.py
48

yep, fixed

This revision was automatically updated to reflect the committed changes.