👍

Yep, eventually... Still need to figure out the right API for the dagster-graphql -> dagster interaction. I'll add some placeholder TODO / comments.

will change

will change

I don't know quite yet... I suppose the default would just use generators instead of lists?

Makes sense. I just think having some helper where don't have to import LastPartitionSelector for the common case will be good for easing into this part of the system

in this current exploration of having partitions as their own top level concept - do schedules need to know about them at all? You could have something in the partitions domain that helps you fill out environment_dict_fn, should_execute, and to-be-added tags_fn to define a schedule based on a partition.

Without this, how would you schedule a partition-based pipeline run?

I agree that it's cleaner not to have overlaps with the environment_dict_fn, should_execute, etc...

I also think we need this to start experimenting w/ partitioned-based schedules

Without this, how would you schedule a partition-based pipeline run?

I was imagining something like

log_date_set = PartitionSetDefinition(
    name='date_partitions',
    pipeline_name='log_partitions',
    partition_fn=date_partition_range(ten_days_ago),
    base_preset='default', # maybe base config instead?
    config_fn=partition_config_fn # merges over base preset
)

# creates a schedule that
#  fires at cron_schedule rate
# selects a partition
# skips if partition already selected otherwise
# applies config from partition selection atop base and runs
log_date_partition = log_date_set.create_schedule(
    name='log_dates',
    cron_schedule='* * * * *',
    selector=... # means to select partition
)

oh, i like that quite a lot!

This is fair, but this would have the consequence of needing to remove instance as an argument from select_partition on a PartitionSelector.

We thread the instance down into select_partition to get the selected_partition to pass into environment_dict_fn_for_partition.

I do agree that it doesn't make sense to have instance as an argument here, so I would lean towards removing it as a param from select_partition to do so.

Do you think it's more important that select_partition has access to the instance?

I think we can figure out a way to add that later if we need to. I'm fine removing it for now.