Skip to content

Conversation

@Taragolis
Copy link
Contributor

@Taragolis Taragolis commented Dec 18, 2023

Related: #35798, the differences is bump to min pendulum 3

As suggestion in https://lists.apache.org/thread/5b3xw87ntl0m00d8jwcz13vnvxhybjdp keep support pendulum 2 for a while, even if it has some problem it might help users to migrate to pendulum 3.

Add --downgrade-pendulum by the same way as it implements into the --downgrade-sqlalchemy so we could also run tests against pendulum 2 for a while.

The only one version of pendulum supported - 2.1.2 with older versions such as 2.0.0 I had a problem with initialised Airflow, there is should not be a big problem because we have 2.1.2 for a long period of time in constraints, and I guess for all Airflow 2.x but I do not checked all constraints only latests.

There is some different behaviour with pendulum 3 vs pendulum 2:

String representation:

Pendulum 2

Has T separator between date and time

>>> import pendulum
>>> str(pendulum.now())
'2023-12-18T23:14:57.788852+04:00'

Pendulum 3

Has (1 whitespace) separator between date and time, same as datetime.datetime

>>> import pendulum
>>> str(pendulum.now())
'2023-12-18 23:14:57.788852+04:00'

UTC serialised for timezone in REST API

Pendulum 2

"timezone":  "Timezone('UTC')",

Pendulum 3

"timezone": "UTC",

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@bolkedebruin
Copy link
Contributor

What happens if serialization (serde) encounters a OLD version of a pendulum timezone? I havent verified it myself but I assume the class names have changed?

@Taragolis
Copy link
Contributor Author

Taragolis commented Dec 18, 2023

Full qualified path is the same. The main difference how UTC timezone works:

  • In pendulum 2 it pendulum.tz.timezone.FixedTimezone, so it deserialise into this type if it serialised in previous version nevermind we serialise offset=0 to UTC
  • In pendulum 3 it pendulum.tz.timezone.Timezone

@Taragolis
Copy link
Contributor Author

Anyway It is a good point to add this kind of tests.

@bolkedebruin
Copy link
Contributor

I like it :-)

@Taragolis
Copy link
Contributor Author

Taragolis commented Dec 23, 2023

Seems like it close to backcompat with 2, just need to resolve remaining static checks

@Taragolis
Copy link
Contributor Author

Seems like all major stuff are resolved, so this PR could be reviewed.
The only things are required is add is newsfragment file

@Taragolis Taragolis marked this pull request as ready for review December 24, 2023 11:55
@Taragolis Taragolis added this to the Airflow 2.8.1 milestone Dec 24, 2023
@potiuk
Copy link
Member

potiuk commented Dec 30, 2023

Looks green. Does it mean we have it ??

@bolkedebruin
Copy link
Contributor

I think we should bite the bullet and start fixing stuff that might be affected. So yes we should have it.

@Taragolis
Copy link
Contributor Author

I will rebase and add release notes today if I feel better

@Taragolis
Copy link
Contributor Author

Just wondering shall we merge it or better to wait for someone else review?

@Taragolis Taragolis force-pushed the pendulum-3 branch 2 times, most recently from ab31119 to 3caa58d Compare January 10, 2024 21:49
@Taragolis
Copy link
Contributor Author

Fortunately All failures after latest rebase not relevant to changes from this PR and should be fixed by #36728

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one looks FANTASTIC

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! LGTM

@Taragolis Taragolis merged commit 2ffa6e4 into main Jan 12, 2024
@Taragolis Taragolis deleted the pendulum-3 branch January 12, 2024 10:27
@potiuk
Copy link
Member

potiuk commented Jan 12, 2024

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

potiuk pushed a commit that referenced this pull request Jan 13, 2024
* Add support of Pendulum 3

* Add backcompat to pendulum 2

* Update airflow/serialization/serialized_objects.py

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

* Add newsfragments

---------

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
(cherry picked from commit 2ffa6e4)
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Jan 15, 2024
ephraimbuddy pushed a commit that referenced this pull request Jan 15, 2024
* Add support of Pendulum 3

* Add backcompat to pendulum 2

* Update airflow/serialization/serialized_objects.py

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

* Add newsfragments

---------

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
(cherry picked from commit 2ffa6e4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:serialization type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants