-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Returning native datetime objects for Bucket/Blob time properties. #807
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
|
Changes Unknown when pulling 9f7d6fc on dhermes:use-native-datetime into * on GoogleCloudPlatform:master*. |
|
I think this is true, but it is an implementation detail so it could break in the future; the discovery document only claims this is in RFC3339 format. That being said, apitools just uses protorpc, which uses datetime exclusively. @craigcitro may be able to comment further; looking at the code we treat the timestamps as DateTimeFields, https://github.com/google/protorpc/blob/master/protorpc/message_types.py#L52, but these are in millisecond format, not RFC3339 and I can't find where it does the string conversion. |
gcloud/storage/test_blob.py
Outdated
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@thobrla The conversion happens in |
|
Ah; the responsible code is actually in protorpc, and that handles both kinds of string formats (using text search and strptime). That seems like a reasonable approach. |
|
Yes, I'm cool doing that (only because we will end up requiring |
|
one more heads-up about timestamps: some newer APIs (eg pubsub) may return you timestamps with even more precision. i.e. down to the nanosecond. (see source here) |
|
@craigcitro WAT? |
|
well, your fix is still good for storage. we can cross the next bridge when we come to it. |
|
@craigcitro You mean this PR is good or the suggestion to use As it were, I can confirm that the timezone support and the option of not having milliseconds (the only extra features) are not necessary. From the discovery type documentation:
|
9f7d6fc to
2645e1d
Compare
|
@tseaver I made an update to the unit tests and also made a slight tweak to the use of PTAL |
|
Changes Unknown when pulling 2645e1d on dhermes:use-native-datetime into * on GoogleCloudPlatform:master*. |
|
LGTM |
Returning native datetime objects for Bucket/Blob time properties.
🤖 I have created a release \*beep\* \*boop\* --- ### [0.42.2](https://www.github.com/googleapis/gapic-generator-python/compare/v0.42.1...v0.42.2) (2021-03-05) ### Bug Fixes * s/grpcAsync/grpc-async for gapic metadata ([#803](https://www.github.com/googleapis/gapic-generator-python/issues/803)) ([96f7864](https://www.github.com/googleapis/gapic-generator-python/commit/96f78640d90cf50c6b525924d14c6afe31874be6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
…age (#807) * chore: upgrade enchant packages in (Owlbot-controlled) docs Docker image Ubuntu Jammy doesn't have the enchant and libenchant1c2a packages. So, after googleapis/synthtool#1422 was merged, which switched the base image of the Dockerfile, it's been unable to install these packages. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Mariatta Wijaya <[email protected]>
* fix: do not use the GAE APIs on gen2+ runtimes Currently, this library uses the App Engine API in all environments if it can be imported successfully. This assumption made sense when the API was only available on gen1, but this is no longer the case. See https://github.com/GoogleCloudPlatform/appengine-python-standard In order to comply with AIP-4115, we must treat GAE gen2+ as a "compute engine equivalent environment" even if the GAE APIs are importable. In other words, google.auth.default() must never return an app_engine.Credental on GAE gen2+.Currently, this library uses the App Engine API in all environments if it can be imported successfully. This assumption made sense when the API was only available on gen1, but this is no longer the case. See https://github.com/GoogleCloudPlatform/appengine-python-standard In order to comply with AIP-4115, we must treat GAE gen2+ as a "compute engine equivalent environment" even if the GAE APIs are importable. In other words, google.auth.default() should not return an app_engine.Credental on GAE gen2+. * blacken Co-authored-by: arithmetic1728 <[email protected]>
Note this was almost in #793 but dropped due to RFC3339 questions.
@craigcitro @thobrla can you vet an assumption for me? Will timestamps returned conform to
I ask because
'2014-11-05T20:34:37Z'is also totally valid according to RFC3339 and I'm wondering if we can get away with usingdatetimealone (and not having to use thestrict-rfc3339library for the full generality of RFC3339).