Skip to content

Commit

Permalink
Remove pendulum from schedules (dagster-io#21391)
Browse files Browse the repository at this point in the history
Noticed that `pendulum` was using up alot of CPU time. This PR uses native `datetime` instead to improve performance inside of schedules, the result is then converted to a `pendulum.datetime` at the end before yielding. 

This slightly improves performance ~10% or so. The next step would be to replace all the rest of the pendulum callsites outside of schedules and stop returning `pendulum.datetime` from our definitions. This would net a large performance improvement around 4x the current status quo.

I also fixed a very small bug where schedules didn't account for non-hour DST transitions.

### Test plan

running buildkite test coverage
  • Loading branch information
sbquinlan authored May 10, 2024
1 parent e075d66 commit dcdb781
Show file tree
Hide file tree
Showing 4 changed files with 224 additions and 271 deletions.
12 changes: 12 additions & 0 deletions python_modules/dagster/dagster/_seven/compat/datetime.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# pyright: reportMissingImports=none
try:
# zoneinfo is python >= 3.9
from zoneinfo import ZoneInfo as _timezone_from_string
except:
from dateutil.tz import gettz as _timezone_from_string
from datetime import tzinfo
from typing import Optional


def timezone_from_string(timezone_name: str) -> Optional[tzinfo]:
return _timezone_from_string(timezone_name)
7 changes: 6 additions & 1 deletion python_modules/dagster/dagster/_seven/compat/pendulum.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ def mock_pendulum_timezone(override_timezone):

def create_pendulum_time(year, month, day, *args, **kwargs):
if "tz" in kwargs and "dst_rule" in kwargs and _IS_PENDULUM_1:
tz = pendulum.timezone(kwargs.pop("tz"))
tz_name_or_info = kwargs.pop("tz")
tz = (
pendulum.timezone(tz_name_or_info)
if isinstance(tz_name_or_info, str)
else tz_name_or_info
)
dst_rule = kwargs.pop("dst_rule")

return pendulum.instance(
Expand Down
Loading

0 comments on commit dcdb781

Please sign in to comment.