https://dagster.io/ logo
#dagster-feedback
Title
# dagster-feedback
s

Spencer Nelson

02/14/2023, 6:32 PM
I would like to have a way to set an end datetime for a time window partition (eg a DailyPartitionDefinition). Sometimes the source data spans a time range that is bounded. I see two ways to do this currently, but both are a little unsatisfactory. I could make a new subclass of
StaticPartitionsDefinition
. This is easy and I know how to do it, but I don’t get any of the cleverness of TimeWindowPartitionMapping, because TimeWindowPartitionMapping’s code implicitly does a bunch of checks like
isinstance(downstream_partitions_def, TimeWindowPartitionsDefinition)
. I like TimeWindowPartitionMapping because it gives a really clear way to break up work. Stuff that’s hard to compute can be done daily, stuff that’s easy to compute can be done monthly, and stuff can remain in time-sorted order, which matters for me. And just grepping the code, it is easy to find dozens of similar
isinstance
checks in _core/execution, _core/definitions/asset_graph.py, and elsewhere - I’m sure I’d be losing out on lots. Or I could make a new subclass of
DailyPartitionDefinition
. But I’m not sure what methods I need to override to get the behavior I want, and it’s not at all clear that the methods I would override are in the public API, so I’m worried my code would break from dagster refactoring.
o

owen

02/14/2023, 7:21 PM
This makes total sense, and is something that TimeWindowPartitionsDefinitions could certainly support out of the box. In terms of subclassing, I wouldn't recommend going down this route. We end up needing to pass these partitions definitions over a serialization boundary, and so custom subclasses will not survive this transformation unfortunately. If you're interested in writing this up as a github issue (or even a PR!) I think this could definitely be useful to others as well. In my mind, this implementation would be an additional (optional) end_date parameter to the TimeWindowPartitionsDefinition.
a

Adam Bloom

02/15/2023, 12:49 PM
2 Views