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

docs: add docs signed_url expiration take default utc #250

Merged
merged 9 commits into from
Aug 27, 2020
16 changes: 12 additions & 4 deletions google/cloud/storage/_signing.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ def get_expiration_seconds_v2(expiration):
"""Convert 'expiration' to a number of seconds in the future.

:type expiration: Union[Integer, datetime.datetime, datetime.timedelta]
: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``.
Copy link
Contributor

Choose a reason for hiding this comment

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

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``.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.


:raises: :exc:`TypeError` when expiration is not a valid type.

Expand Down Expand Up @@ -123,7 +125,9 @@ def get_expiration_seconds_v4(expiration):
"""Convert 'expiration' to a number of seconds offset from the current time.

:type expiration: Union[Integer, datetime.datetime, datetime.timedelta]
: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``.

:raises: :exc:`TypeError` when expiration is not a valid type.
:raises: :exc:`ValueError` when expiration is too large.
Expand Down Expand Up @@ -299,7 +303,9 @@ def generate_signed_url_v2(
Caller should have already URL-encoded the value.

:type expiration: Union[Integer, datetime.datetime, datetime.timedelta]
: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``.

:type api_access_endpoint: str
:param api_access_endpoint: (Optional) URI base. Defaults to empty string.
Expand Down Expand Up @@ -461,7 +467,9 @@ def generate_signed_url_v4(
Caller should have already URL-encoded the value.

:type expiration: Union[Integer, datetime.datetime, datetime.timedelta]
: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``.

:type api_access_endpoint: str
:param api_access_endpoint: (Optional) URI base. Defaults to
Expand Down
4 changes: 3 additions & 1 deletion google/cloud/storage/blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,9 @@ def generate_signed_url(
log in.

:type expiration: Union[Integer, datetime.datetime, datetime.timedelta]
: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``.

:type api_access_endpoint: str
:param api_access_endpoint: (Optional) URI base.
Expand Down
4 changes: 3 additions & 1 deletion google/cloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -2955,7 +2955,9 @@ def generate_signed_url(
log in.

:type expiration: Union[Integer, datetime.datetime, datetime.timedelta]
: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``.

:type api_access_endpoint: str
:param api_access_endpoint: (Optional) URI base.
Expand Down
8 changes: 6 additions & 2 deletions google/cloud/storage/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,9 @@ def generate_signed_post_policy_v4(
:param blob_name: Object name.

:type expiration: Union[Integer, datetime.datetime, datetime.timedelta]
:param expiration: Policy expiration time.
:param expiration: Policy expiration time. If expiration in datetime,
then need to pass timezone with it. Default value of
``tzinfo`` is ``UTC``.

:type conditions: list
:param conditions: (Optional) List of POST policy conditions, which are
Expand Down Expand Up @@ -1004,11 +1006,13 @@ def generate_signed_post_policy_v4(
Generate signed POST policy and upload a file.

>>> from google.cloud import storage
>>> import pytz
>>> client = storage.Client()
>>> tz = pytz.timezone('America/New_York')
>>> policy = client.generate_signed_post_policy_v4(
"bucket-name",
"blob-name",
expiration=datetime.datetime(2020, 3, 17),
expiration=datetime.datetime(2020, 3, 17, tzinfo=tz),
conditions=[
["content-length-range", 0, 255]
],
Expand Down