docs: add docs signed_url expiration take default utc #250
Conversation
google/cloud/storage/_signing.py
Outdated
| :param expiration: Point in time when the signed URL should expire. | ||
| :param expiration: Point in time when the signed URL should expire. If | ||
| expiration in datetime, then need to pass timezone | ||
| with it. Default value of ``tzinfo`` is ``UTC``. |
There was a problem hiding this comment.
The default value of the tzinfo argument to datetime.datetime is None, resulting in a "naive" datetime instance: the comment should state clearly that we forcibly convert such instances to UTC. E.g.:
:param expiration: Point in time when the signed URL should expire. If
a `datetime` instance is passed without an explicit
`tzinfo` set, it will be converted to ``UTC``.There was a problem hiding this comment.
Rather than "it will be converted to" let's use "it will be assumed to be". This is a huge footgun; a user who sees the term "converted" will assume that we will do the right thing in the default case, whereas in fact we will almost always do the wrong thing.
|
Cancel that, I just saw that the test issue is resolved in #251. Thanks for that! Leaving review on "changes requested" for the docstring update as per my comment; lgtm other than that. |
|
@HemangChothani I would merge my suggestions to address the concern expressed by @andrewsg , but you have the "maintainers can push changes" option turned off. |
…into storage_issue_244
Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com> Co-authored-by: Tres Seaver <tseaver@palladion.com>
Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com> Co-authored-by: Tres Seaver <tseaver@palladion.com>
Fixes #244