Page MenuHomePhabricator

Create resource for partitions

Authored by sashank on Dec 9 2019, 10:27 PM.



This diff introduces a partition_resource, which grabs partition_set_name and partition_name off of the pipeline run tags and provides the correct Partition object as a resource.

Tradeoff of this approach is that we can't execute pipelines that use the partition resource, since they won't have the correct tags on the pipeline run. We'll need to build a UI in the config editor that let's us edit tags, and also automatically fill out config and tags by selecting a partition.

Test Plan


Diff Detail

R1 dagster
Lint OK
No Unit Test Coverage

Event Timeline

sashank updated this revision to Diff 7338.Dec 9 2019, 10:27 PM
sashank created this revision.


sashank edited the summary of this revision. (Show Details)Dec 9 2019, 10:28 PM
sashank edited the summary of this revision. (Show Details)
sashank edited the summary of this revision. (Show Details)
sashank added inline comments.Dec 9 2019, 10:32 PM

This works when executing a pipeline through dagit, but get_handle returns (None, None) when running the pipeline using execute_pipeline.


So, will need to figure out another way to thread the execution_target_handle through to here to get the partition_handle

sashank added a reviewer: max.Dec 9 2019, 10:33 PM
sashank updated this revision to Diff 7340.Dec 9 2019, 10:35 PM

Fix imports

sashank planned changes to this revision.EditedDec 9 2019, 11:10 PM

After discussion with @max: Instead of passing in the partition_set_definition_name and partition_name through resource config, we can grab these two things off tags. We already have the dagster/partition_name tag, so we should add the dagster/partition_set_definition_name tag.

This approach would come at the cost of not having a way to execute pipeline runs for a partition through dagit. But we could soon have a Partition selector (just like we have a preset selector) that would add the appropriate tags and pre-populate the config for the partition.

sashank planned changes to this revision.Dec 10 2019, 7:18 PM
sashank updated this revision to Diff 7525.Dec 10 2019, 11:05 PM

Update tests, use tags to get partition in partition_resource

sashank edited the summary of this revision. (Show Details)Dec 10 2019, 11:16 PM
sashank updated this revision to Diff 7531.Dec 10 2019, 11:21 PM
sashank edited the summary of this revision. (Show Details)


sashank updated this revision to Diff 7533.Dec 10 2019, 11:25 PM


sashank updated this revision to Diff 7537.Dec 10 2019, 11:33 PM

Add missing files + lint

sashank added inline comments.Dec 10 2019, 11:39 PM

Not sure if this should return None or error out

sashank updated this revision to Diff 7549.Dec 11 2019, 12:12 AM

rerun tests

prha added inline comments.Dec 11 2019, 12:18 AM

Yeah, I think we should probably raise for all of these.


This should probably raise?

prha requested changes to this revision.Dec 11 2019, 12:24 AM

returning to your queue

This revision now requires changes to proceed.Dec 11 2019, 12:24 AM
sashank updated this revision to Diff 7557.Dec 11 2019, 12:28 AM

raise errors when partition_resource cannot be initialized

max resigned from this revision.Dec 11 2019, 10:02 PM

i think i've said my piece on this being useful in the edge case / for introspection -- let's not foreground use of this resource in ordinary examples / usage patterns, we want people to be able to slice config space independently of writing solid logic

alangenfeld added inline comments.Dec 12 2019, 4:22 PM

why are the PartitionSelectors classes?

prha added inline comments.Dec 16 2019, 4:25 PM

Yeah, I envisioned this being more complicated than it actually is... they should actually just be functions.

alangenfeld accepted this revision.Dec 16 2019, 4:31 PM

I think this seems fine to exist

sashank removed a reviewer: prha.Dec 17 2019, 12:22 AM
This revision is now accepted and ready to land.Dec 17 2019, 12:22 AM
prha accepted this revision.Dec 17 2019, 12:31 AM
sashank added a subscriber: max.Jan 24 2020, 8:40 PM

After working more with partitions, I think introducing this would encourage implementing partitioned-based schedules using the anti-pattern @max mentioned. We can hold off on it until we see a clear need for a partition resource.

sashank abandoned this revision.Jan 24 2020, 8:40 PM