Skip to content

Bugfix datetime serialization utc zero offset #1016

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

Conversation

stefan-jozefowicz-bayer
Copy link
Contributor

Proposed Changes:

For inclusion in changelog (if applicable):

Not intended for changelog:

  1. Change how timezone aware datetimes with UTC tz are converted to datetime string

Diff of User Interface

Old behavior:
passing a timezone aware datetime from the UTC zone to some of the request constructors resulted in malformed datetime strings, e.g: 2022-06-01T01:00:00+00:00Z - i.e. strings that have BOTH +00:00 and Z at the end.

At the root of it was value.utcoffset() >> timedelta(hours=0) evaluating as False even though there is a defined offset here:

def _datetime_to_rfc3339(value: datetime) -> str:
    """Converts the datetime to an RFC3339 string"""
    iso = value.isoformat()
    if not value.utcoffset():
        # rfc3339 needs a Z if there is no timezone offset
        iso += 'Z'
    return iso

New behavior:
The Z is tagged on only if the utcoffset() is None.

When the TZ is UTC the dt string is:
2022-06-01T01:00:00+00:00

The previous test setup for timezone aware datetimes obfuscated this bug, so I changed the tests as well.

PR Checklist:

  • This PR is as small and focused as possible
  • If this PR includes proposed changes for inclusion in the changelog, the title of this PR summarizes those changes and is ready for inclusion in the Changelog.
  • I have updated docstrings for function changes and docs in the 'docs' folder for user interface / behavior changes
  • This PR does not break any examples or I have updated them

(Optional) @mentions for Notifications:

@pl-gideon pl-gideon self-requested a review November 22, 2023 21:05
@pl-gideon pl-gideon changed the base branch from release-2.2 to main November 23, 2023 20:54
Copy link
Member

@pl-gideon pl-gideon left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this issue!

@pl-gideon pl-gideon merged commit 4873a48 into planetlabs:main Nov 23, 2023
@adamweiner adamweiner mentioned this pull request Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants