DAG serialization broken because of pendulum

[cansjt]

Apache Airflow version

2.2.2 (latest released)

Operating System

Debian

Versions of Apache Airflow Providers

N/A

Deployment

Other 3rd-party Helm chart

Deployment details

N/A

What happened

After upgrading from 2.1.3 to 2.2.2, Airflow is no longer capable of serializing some dags. It fails with the exception:

Traceback (most recent call last):
  File \"/usr/local/lib/python3.9/site-packages/airflow/serialization/serialized_objects.py\", line 817, in serialize_dag
    serialize_dag = cls.serialize_to_json(dag, cls._decorated_fields)
  File \"/usr/local/lib/python3.9/site-packages/airflow/serialization/serialized_objects.py\", line 269, in serialize_to_json
    serialized_object[key] = _encode_timetable(value)
  File \"/usr/local/lib/python3.9/site-packages/airflow/serialization/serialized_objects.py\", line 151, in _encode_timetable
    return {\"__type\": importable_string, \"__var\": var.serialize()}
  File \"/usr/local/lib/python3.9/site-packages/airflow/timetables/interval.py\", line 155, in serialize
    return {\"expression\": self._expression, \"timezone\": encode_timezone(self._timezone)}
  File \"/usr/local/lib/python3.9/site-packages/airflow/serialization/serialized_objects.py\", line 116, in encode_timezone
    raise ValueError(f\"DAG timezone should be a pendulum.tz.Timezone, not {var!r}\")
ValueError: DAG timezone should be a pendulum.tz.Timezone, not datetime.timezone.utc

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File \"/usr/local/lib/python3.9/site-packages/airflow/models/dagbag.py\", line 591, in _serialize_dag_capturing_errors
    dag_was_updated = SerializedDagModel.write_dag(
  File \"/usr/local/lib/python3.9/site-packages/airflow/utils/session.py\", line 67, in wrapper
    return func(*args, **kwargs)
  File \"/usr/local/lib/python3.9/site-packages/airflow/models/serialized_dag.py\", line 136, in write_dag
    new_serialized_dag = cls(dag)
  File \"<string>\", line 4, in __init__
  File \"/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/state.py\", line 433, in _initialize_instance
    manager.dispatch.init_failure(self, args, kwargs)
  File \"/usr/local/lib/python3.9/site-packages/sqlalchemy/util/langhelpers.py\", line 68, in __exit__
    compat.raise_(
  File \"/usr/local/lib/python3.9/site-packages/sqlalchemy/util/compat.py\", line 182, in raise_
    raise exception
  File \"/usr/local/lib/python3.9/site-packages/sqlalchemy/orm/state.py\", line 430, in _initialize_instance
    return manager.original_init(*mixed[1:], **kwargs)
  File \"/usr/local/lib/python3.9/site-packages/airflow/models/serialized_dag.py\", line 95, in __init__
    self.data = SerializedDAG.to_dict(dag)
  File \"/usr/local/lib/python3.9/site-packages/airflow/serialization/serialized_objects.py\", line 935, in to_dict
    json_dict = {\"__version\": cls.SERIALIZER_VERSION, \"dag\": cls.serialize_dag(var)}
  File \"/usr/local/lib/python3.9/site-packages/airflow/serialization/serialized_objects.py\", line 847, in serialize_dag
    raise SerializationError(f'Failed to serialize DAG {dag.dag_id!r}: {e}')
airflow.exceptions.SerializationError: Failed to serialize DAG 'preview_meta_eventad_registrations': DAG timezone should be a pendulum.tz.Timezone, not datetime.timezone.utc

The start date was provide to the DAG initializer as datetime.datetime(2020, 11, 1, 0, 0, tzinfo=datetime.timezone.utc), which, FYI, is the result of the parsing of a YAML file.

What you expected to happen

Airflow should be capable of serializing the DAG without any error.

How to reproduce

Create a DAG with a start date as follows:

from datetime import datetime, timezone
from airflow.models import DAG

DAG('whatever', start_date=datetime(2021,12,5, 0, 0, 0, tzinfo=timezone.utc, ...)

And enable DAG serialization in Airflow's configuration (though this is the default now, no more store_serialized_dags option in the configuration reference).

Anything else

This is similar but yet slightly different from #16613.
May also be related to #19450.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[potiuk]

I believe this is deliberate. The error tells what to do - change the timezone to use pendulum.

If you look here: https://airflow.apache.org/docs/apache-airflow/stable/timezone.html#time-zone-aware-dags, you are supposed to use pendulum timezones (and it's been there for quite a while when we expected pendulum not datetime). While previously it coudl accidentally work with datetime timezones, it was, I believe, not intended. So please convert your DAGs to use the pendulum timezones and it should be fixed.

@uranusjr - am I right? Should we close it?

[cansjt]

As far as I am concerned if this is deliberate this is not a great choice. As mentioned this date comes from the output of the Parsing of a YAML file. For now we implemented a work around: intercept the output of the parsing before passing it to Airflow.

But, to me, using pendulum is a choice internal to Airflow. At it's interface Airflow should be capable of accepting anything that comes from Python standard library. And should not impose to use anything extra to deal with something as simple as dates.

To the least the DAG class should validate properly validate its input, as you put new constraints on the input it accepts.

Also this change is not really documented (still states here start_date should be a datetime.datetime), and all the examples I could find accept datetime as input. And again it was working fine. Something that worked, but does no more, should fit anyone's definition of a bug (or so I hope).

[cansjt]

For those who might wonder, there is this documentation section. Maybe the documentation of the DAG class should be updated to reference this section so people are more aware of it.

[potiuk]

Something that worked, but does no more, should fit anyone's definition of a bug (or so I hope).

That's a very simplistic view.. Sometimes things work accidentally - especially if the implementation doesn't follow the documentation. So this is by far not a universal definition of bug that anyone should follow. It's not "0-1" definition by far

But I am not sure how it was in this case, so I will revert to others to comment. I am not sure what was the case here - otherwise I'd have closed this or moved to a discussion straight away.

[kaxil]

Airflow has a utility for it that you can use to make sure you don't need to use anything else.

from airflow.utils import timezone

now = timezone.utcnow()
a_date = timezone.datetime(2017, 1, 1)

https://airflow.apache.org/docs/apache-airflow/2.2.2/timezone.html#naive-and-aware-datetime-objects

Airflow have relied on Pendulum for as long as I can remember and we make it clear on https://airflow.apache.org/docs/apache-airflow/2.2.2/timezone.html on why we picked pendulum instead of pytz too.

However, if you have any suggestions on how we can make it clearer, we are happy to hear your thoughts

[potiuk]

Yeah. Concur with @kaxil - maybe there are some ways we can make it clearer. I will convert this into discussion now - but maybe we can continue discussing what can be done to improve there ((and @cansjt -> maybe you can even directly create a PR to add clear the confusion in the examples you pointed out ? Airflow has > 1800 contributors, so that seems like an easy way to become one as well.

[uranusjr]

To clarify, #19450 is not strongly related. datetime.timezone is offset-based, while that issue is for introducing support to zoneinfo.ZoneInfo, which is based on the IANA database like Pendulum timezones, and can be serialised in the same way. Offset-based timezones like datetime.timezone must be serialised (and more importantly, deserialised) in a different format, and I have not figure out a good way to do it reliably without needing to access private attributes.

[potiuk]

I think this way there is a higher chance that users will be able to understand why tthis is important and solve it on their own rather than opening an issue. Simple "Read more [here] to understand why you need to use pendulum timezones".

[potiuk]

Happy to draft something like that if you want help with that @uranusjr

[uranusjr]

Where do you think is a good place for this information? Does it fit in Airflow’s documentation? I can probably write some prose on this.

[potiuk]

I think we alreayd have good place https://airflow.apache.org/docs/apache-airflow/2.2.2/timezone.html with just speciic note linking to the right place in the doc in the error message.

[uranusjr]

Makes sense. I’ll try to write something.

[jceresini]

I ran into this after upgrading from 2.1.4 to 2.2.3 using Google's Cloud Composer (basically a managed airflow installation). I found this issue pretty quickly based on the error on some of our DAGs.

I wanted to provide my thoughts.

Much of the documentation and examples I've seen use naive datetimes (ex: start_date=datetime(2021, 1, 1)). These are the examples I started with recently. There's one such example on the FAQ page.

As mentioned in a previous comment in this thread, the DAG documentation states the start_time is a datetime.datetime object, but doesn't mention there's a non-standard-library timezone implementation required. I think noting that requirement in that part of the documentation would be helpful for others.

It's also odd (to me) to have a specific timezone implementation, but to continue to allow naive datetimes. If airflow always enforced tz-aware datetimes, documentation and examples would need to get updated to continue working. It would likely be a much harder transition than the one from Airflow 2.1 to 2.2, and impact a lot of people, so I would hope a change like that wouldn't happen outside of a major release.

There's a FAQ page, which includes the question "What is the deal with start_time?" and it doesn't mention pendulum once. The tutorial also doesn't mention pendulum, or the words "aware", "naive", "timezone", "tz", etc. Tutorials and FAQs are the places most new users are going to start out with any new project.

I know its not directly related to airflow, but Google's Cloud Composer is also driving new uses to Airflow. Their documentation also uses naive datetimes in their examples, and I havent seen mention of pendulum in their documentation. I plan on opening a support case with google and mentioning this issue in the hopes that they take some time to augment their documentation.

[potiuk]

Thanks @jceresini - i think you are totally right it was not clear and stressed enough. The change was good, the "coincidence" it run before (even though for a long time we warned and it's even nicely described here: https://airflow.apache.org/docs/apache-airflow/stable/timezone.html?highlight=timezone#time-zone-aware-dags) was unfortunate.

None of our examples (including the one that you pointed out) shown the timezone-aware DAG without pendulum, and we told the users to use pendulum, but you are completely right that this could have led you to (very wrong actually) using of the stdlib.

And really cool from your side that you pointed out to the areas that you looked at when you were looking for it. That helped me to identify the places to improve. Thanks for that! I've just createad a PR to clarify that: #21646

BTW. Next time I really encourage people like you to not even report problems like that but simply create docuementation PRs. This is actually super-easy. Just click on "Suggest a change on this page" and you can propose a change to our documentation as a PR. It's as easy as creating and issue or commening on a discussion. And you can become one of the almost 2000 contributors to Airflow. this way.

Really looking forward for more vigilance like that - our documentation might be imperfect, but if users likely you not only report such issues but also provide fixes to the docs immediately, I am sure we can improve it a lot.

[potiuk]

Since #21646 has been merged. @jceresini - woudl you find some time to review the changes and see if they address your comments? Any comment here is good. I really hate to implement something as response of complaints and never hear back from the people who complain when it is fixed. Did it fix your problems ? or not ? if not -why not?

It would really be great to at least thank to those who respond to your needs if they do, especially if you could not wait to provide your thoughts before.

Looking forward to hearing from you.