bolkedebruin commented on code in PR #34744:
URL: https://github.com/apache/airflow/pull/34744#discussion_r1347009859


##########
airflow/utils/timezone.py:
##########
@@ -18,14 +18,20 @@
 from __future__ import annotations
 
 import datetime as dt
+from functools import lru_cache
 from typing import overload
 
 import pendulum
 from dateutil.relativedelta import relativedelta
 from pendulum.datetime import DateTime
+from pendulum.tz import fixed_timezone
+from pendulum.tz.timezone import FixedTimezone, Timezone
 
-# UTC time zone as a tzinfo instance.
-utc = pendulum.tz.timezone("UTC")
+# UTC time zone as a FixedTimezone instance (subclass of tzinfo)
+# This type uses for compatibility with type provided by pendulum 2.x
+# - in pendulum 2.x ``pendulum.tz.timezone`` returns FixedTimezone
+# - in pendulum 3.x ``pendulum.timezone`` returns Timezone
+utc = FixedTimezone(offset=0, name="UTC")

Review Comment:
   is there an issue moving to Pendulum 3 UTC? So moving from FixedTimezone to 
Timezone? FixedTimezone derives from Timezone.



##########
tests/models/test_dag.py:
##########
@@ -674,8 +674,8 @@ def test_following_previous_schedule(self):
         """
         Make sure DST transitions are properly observed
         """
-        local_tz = pendulum.timezone("Europe/Zurich")
-        start = local_tz.convert(datetime.datetime(2018, 10, 28, 2, 55), 
dst_rule=pendulum.PRE_TRANSITION)
+        local_tz = Timezone("Europe/Zurich")
+        start = local_tz.convert(datetime.datetime(2018, 10, 28, 2, 55, 
fold=0))

Review Comment:
   maybe redefine PRE_TRANSITION so it is more clear what fold=0 means?



##########
airflow/utils/timezone.py:
##########
@@ -273,3 +279,24 @@ def _format_part(key: str) -> str:
     if not joined:
         return "<1s"
     return joined
+
+
+@lru_cache(maxsize=None)
+def parse_timezone(name: str | int) -> Timezone | FixedTimezone:

Review Comment:
   just `Timezone` should be sufficient? What about the parameter `name`? That 
looks a bit awkward if you are checking for an `int`. What about `tz` and 
making it part of the docstring?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to