Page MenuHomePhabricator

Add environment_dict_fn argument to ScheduleDefinition
ClosedPublic

Authored by sashank on Mon, Oct 21, 5:55 AM.

Details

Reviewers
schrockn
Group Reviewers
Restricted Project
Commits
R1:6094632ea562: Add environment_dict_fn argument to ScheduleDefinition
Summary

This diff adds a environment_dict_fn parameter to ScheduleDefinition. This can be used instead of the environment_dict parameter, and should be set to a function that returns an environment_dict.

This will help create date partitioned schedules that are based on config.

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

sashank created this revision.Mon, Oct 21, 5:55 AM
sashank edited the summary of this revision. (Show Details)Mon, Oct 21, 10:16 PM
sashank edited the test plan for this revision. (Show Details)
Harbormaster completed remote builds in B4763: Diff 5948.
sashank updated this revision to Diff 5952.Mon, Oct 21, 10:22 PM
sashank edited the summary of this revision. (Show Details)

Add invalid definition error

sashank updated this revision to Diff 5956.Tue, Oct 22, 12:36 AM

add tests

sashank edited the test plan for this revision. (Show Details)
sashank added a reviewer: Restricted Project.
sashank updated this revision to Diff 5970.Tue, Oct 22, 5:43 PM

update test

schrockn requested changes to this revision.Tue, Oct 22, 10:50 PM
schrockn added a subscriber: schrockn.

I'd like to see a full RFC of date-time partition or a plan of action. We are probably going to have to thread through some sort of context through to the environment_dict_fn and it would be good to see that end-to-end before moving forward with the incremental diffs.

python_modules/dagster/dagster/check/__init__.py
680

this is a dupe of opt_callable_param. Can you make an issue to consolidate those two check functions? (as well as fn_param versus callable_param)?

This revision now requires changes to proceed.Tue, Oct 22, 10:50 PM
sashank added a comment.EditedTue, Oct 22, 11:24 PM

Here's the example of the date-time partitioned schedule (next diff in the stack): https://dagster.phacility.com/D1311

sashank added inline comments.Tue, Oct 22, 11:28 PM
python_modules/dagster/dagster/check/__init__.py
680

ah didn't realize it already existed. consolidated them in this diff, since there was only one callsite for fn_param.

sashank updated this revision to Diff 5984.EditedTue, Oct 22, 11:31 PM

consolidate fn_param and callable_param

schrockn accepted this revision.Wed, Oct 23, 2:12 AM

cool. i'm excited about this direction

This revision is now accepted and ready to land.Wed, Oct 23, 2:12 AM