ecerulm commented on a change in pull request #16678:
URL: https://github.com/apache/airflow/pull/16678#discussion_r660624826
##########
File path: airflow/models/dag.py
##########
@@ -304,14 +339,15 @@ def __init__(
# set timezone from start_date
if start_date and start_date.tzinfo:
- self.timezone = start_date.tzinfo
+ self._schedule_timezone = _get_tzname_or_offset(start_date.tzinfo)
elif 'start_date' in self.default_args and
self.default_args['start_date']:
if isinstance(self.default_args['start_date'], str):
self.default_args['start_date'] =
timezone.parse(self.default_args['start_date'])
- self.timezone = self.default_args['start_date'].tzinfo
+ if self.default_args['start_date'] and
self.default_args['start_date'].tzinfo:
+ self._schedule_timezone =
_get_tzname_or_offset(self.default_args['start_date'].tzinfo)
- if not hasattr(self, 'timezone') or not self.timezone:
- self.timezone = settings.TIMEZONE
+ if not hasattr(self, '_schedule_timezone'):
+ self._schedule_timezone = _get_tzname_or_offset(settings.TIMEZONE)
Review comment:
sure
##########
File path: airflow/models/dag.py
##########
@@ -302,16 +304,20 @@ def __init__(
self.fileloc = back.f_code.co_filename if back else ""
self.task_dict: Dict[str, BaseOperator] = {}
- # set timezone from start_date
+ # set timezone from schedule_timezone or start_date
+ if schedule_timezone:
+ self.schedule_timezone =
self._get_tzname_or_offset(schedule_timezone)
if start_date and start_date.tzinfo:
- self.timezone = start_date.tzinfo
+ self.schedule_timezone =
self._get_tzname_or_offset(start_date.tzinfo)
+ # self.timezone = start_date.tzinfo
elif 'start_date' in self.default_args and
self.default_args['start_date']:
if isinstance(self.default_args['start_date'], str):
self.default_args['start_date'] =
timezone.parse(self.default_args['start_date'])
- self.timezone = self.default_args['start_date'].tzinfo
+ if self.default_args['start_date'] and
self.default_args['start_date'].tzinfo:
+ self.schedule_timezone =
self._get_tzname_or_offset(self.default_args['start_date'].tzinfo)
- if not hasattr(self, 'timezone') or not self.timezone:
- self.timezone = settings.TIMEZONE
+ if not hasattr(self, 'schedule_timezone'):
+ self.schedule_timezone =
self._get_tzname_or_offset(settings.TIMEZONE)
Review comment:
I left dag.py untouched now
##########
File path: airflow/utils/timezone.py
##########
@@ -184,3 +185,38 @@ def coerce_datetime(v: Union[None, dt.datetime, DateTime])
-> Optional[DateTime]
if v.tzinfo is None:
v = make_aware(v)
return pendulum.instance(v)
+
+
+def _get_tzname_or_offset(obj):
+ if _is_valid_pendulum_tzname(obj):
+ return obj
+ if hasattr(obj, 'name'):
+ candidate = obj.name
+ if _is_valid_pendulum_tzname(candidate):
+ return candidate
+ try:
+ candidate = obj.tzname(dt.datetime.now(dt.timezone.utc))
+ if _is_valid_pendulum_tzname(candidate):
+ return candidate
+ except (NotImplementedError, ValueError):
+ pass
+
+ try:
+ candidate =
int(obj.utcoffset(dt.datetime.now(dt.timezone.utc)).total_seconds())
Review comment:
@ashb do we want to do this or not? at this point there is no real
guarantee that the `datetime.tzinfo` that is really a fixed offset timezone.
There is no good way to check if it really is. I can do an `isinstance(obj,
pendulum.tz.timezone.FixedTimezone)` that works only for pendulum timezones.
The risk is that someone uses some custom `datetime.tzinfo` with a custom
name with no equivalent in pendulum, and this serialization logic will reduce
that custom tzinfo to a fixedoffset timezone (which will have a different
behaviour than the original dag.timezone).
##########
File path: airflow/utils/timezone.py
##########
@@ -184,3 +185,38 @@ def coerce_datetime(v: Union[None, dt.datetime, DateTime])
-> Optional[DateTime]
if v.tzinfo is None:
v = make_aware(v)
return pendulum.instance(v)
+
+
+def _get_tzname_or_offset(obj):
+ if _is_valid_pendulum_tzname(obj):
+ return obj
+ if hasattr(obj, 'name'):
+ candidate = obj.name
+ if _is_valid_pendulum_tzname(candidate):
+ return candidate
+ try:
+ candidate = obj.tzname(dt.datetime.now(dt.timezone.utc))
+ if _is_valid_pendulum_tzname(candidate):
+ return candidate
+ except (NotImplementedError, ValueError):
+ pass
+
+ try:
+ candidate =
int(obj.utcoffset(dt.datetime.now(dt.timezone.utc)).total_seconds())
Review comment:
@ashb , @uranusjr do we want to do this or not? at this point there is
no real guarantee that the `datetime.tzinfo` that is really a fixed offset
timezone. There is no good way to check if it really is. I can do an
`isinstance(obj, pendulum.tz.timezone.FixedTimezone)` that works only for
pendulum timezones.
The risk is that someone uses some custom `datetime.tzinfo` with a custom
name with no equivalent in pendulum, and this serialization logic will reduce
that custom tzinfo to a fixedoffset timezone (which will have a different
behaviour than the original dag.timezone).
--
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]
