-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cdk unit test #33572
Fix cdk unit test #33572
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with this change. However, it makes me very worried about how we use the pendulum library. str(<pendulum.Datetime>)
is kind of scary as __str__
is defined as an informal and nicely printable string
https://docs.python.org/3/reference/datamodel.html#object.__str__
I agree @maxi297 , I first wanted to fix it that route by explicitly serializing to an iso string, but it spiraled into a pretty big change and I wanted to unblock PRs quickly: #33571 I opened an issue to solve this in a better way: https://github.com/airbytehq/airbyte-internal-issues/issues/2356 |
Pendulum got updated to 3.0: https://pypi.org/project/pendulum/#history
As we don't pin the version, this caused some errors. This is setting it to lower than 3.0, but we should upgrade in time