Page MenuHomePhabricator

Add environment_dict_fn argument to ScheduleDefinition
ClosedPublic

Authored by sashank on Oct 21 2019, 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 edited the test plan for this revision. (Show Details)
sashank edited the summary of this revision. (Show Details)

Add invalid definition error

sashank edited the test plan for this revision. (Show Details)
sashank added a reviewer: Restricted Project.
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
679

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.Oct 22 2019, 10:50 PM

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

python_modules/dagster/dagster/check/__init__.py
679

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

consolidate fn_param and callable_param

cool. i'm excited about this direction

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