Skip to content
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: correctly decode times without microseconds #375

Merged
merged 5 commits into from Feb 10, 2021

Conversation

@tritone
Copy link
Contributor

@tritone tritone commented Feb 9, 2021

Currently custom_time is not being decoded correctly if the value
has a zero in the microseconds field. This fixes the issue for
custom_time as well as elsewhere by replacing _datetime_to_rfc3339
with _rfc3339_nanos_to_datetime.

Fixes #363

Currently custom_time is not being decoded correctly if the value
has a zero in the microseconds field. This fixes the issue for
custom_time as well as elsewhere by replacing _datetime_to_rfc3339
with _rfc3339_nanos_to_datetime.

Fixes googleapis#363
@tritone tritone force-pushed the customtime-micros branch from 0de2a21 to d2e1fc6 Feb 9, 2021
@tritone tritone changed the title fix: custom_time decoding without microseconds fix: correctly decode times without microseconds Feb 9, 2021
@tritone tritone marked this pull request as ready for review Feb 9, 2021
@tritone tritone requested a review from Feb 9, 2021
@frankyn frankyn requested a review from as a code owner Feb 9, 2021
frankyn
frankyn approved these changes Feb 9, 2021
@tseaver
Copy link
Contributor

@tseaver tseaver commented Feb 9, 2021

@tritone, @frankyn Seems like we need one or more unit test which assert this change in behavior?

@frankyn
Copy link
Member

@frankyn frankyn commented Feb 9, 2021

Thanks @tseaver. Did we miss a coverage check?

@tritone
Copy link
Contributor Author

@tritone tritone commented Feb 9, 2021

@tritone, @frankyn Seems like we need one or more unit test which assert this change in behavior?

The system test does cover the case that was failing before and ensures that we're correctly parsing the timestamp that GCS returns. I haven't dug into how this code is unit tested currently but I can look into it if you think it would help.

@gcf-merge-on-green gcf-merge-on-green bot merged commit 37a1eb5 into googleapis:master Feb 10, 2021
5 checks passed
cojenco added a commit to cojenco/python-storage that referenced this issue Oct 13, 2021
Currently custom_time is not being decoded correctly if the value
has a zero in the microseconds field. This fixes the issue for
custom_time as well as elsewhere by replacing _datetime_to_rfc3339
with _rfc3339_nanos_to_datetime.

Fixes googleapis#363
cojenco added a commit to cojenco/python-storage that referenced this issue Oct 13, 2021
Currently custom_time is not being decoded correctly if the value
has a zero in the microseconds field. This fixes the issue for
custom_time as well as elsewhere by replacing _datetime_to_rfc3339
with _rfc3339_nanos_to_datetime.

Fixes googleapis#363
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants