Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

fix(storage): use OrderedDict while encoding POST policy - #95

Merged
frankyn merged 3 commits into
googleapis:masterfrom
MaxxleLLC:post_policies_flaky_fix
Apr 1, 2020
Merged

fix(storage): use OrderedDict while encoding POST policy#95
frankyn merged 3 commits into
googleapis:masterfrom
MaxxleLLC:post_policies_flaky_fix

Conversation

@IlyaFaer

Copy link
Copy Markdown

Towards #64 (comment)
Sometimes policy losses it's order while encoding into JSON. Adding OrderedDict() to fix this.

@IlyaFaerIlyaFaer added the api: storage Issues related to the googleapis/python-storage API. label Apr 1, 2020
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 1, 2020
@IlyaFaerIlyaFaer changed the title use OrderedDict() while encoding POST policyfix(storage): use OrderedDict() while encoding POST policyApr 1, 2020
"expiration": policy_expires.isoformat() + "Z",
}.items()
)
),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

expiration sometimes encoded before conditions - in such a cases tests are failing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a little concerned about imposing order in signing code given it's only used for conformance tests.

However, it doesn't change the outcome of expectation and will work as expected as a user.

out_data = test_data["policyOutput"]

decoded_policy = base64.b64decode(fields["policy"]).decode("unicode_escape")
assert decoded_policy == out_data["expectedDecodedPolicy"]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved decoded_policy assert to the top: if something gone wrong in policy, it'll be easier to see it in decoded state - probably will save some time on debugging

@IlyaFaer
IlyaFaer requested a review from frankynApril 1, 2020 20:27
@IlyaFaer
IlyaFaer marked this pull request as ready for review April 1, 2020 20:28
@IlyaFaer

IlyaFaer commented Apr 1, 2020

Copy link
Copy Markdown
Author

@frankyn, kokoro failed in TestStorageCompose.test_compose_create_new_blob_wo_content_type. Not related I assume

_______ TestStorageCompose.test_compose_create_new_blob_wo_content_type ________
self = <test_system.TestStorageCompose testMethod=test_compose_create_new_blob_wo_content_type>
def test_compose_create_new_blob_wo_content_type(self):
SOURCE_1 = b"AAA\n"
source_1 = self.bucket.blob("source-1")
> source_1.upload_from_string(SOURCE_1)
tests/system/test_system.py:1176:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
google/cloud/storage/blob.py:1435: in upload_from_string
self.upload_from_file(
google/cloud/storage/blob.py:1344: in upload_from_file
_raise_from_invalid_response(exc)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
error = InvalidResponse('Request failed with status code', 403, 'Expected one of', <HTTPStatus.OK: 200>)
def _raise_from_invalid_response(error):
"""Re-wrap and raise an ``InvalidResponse`` exception.
:type error: :exc:`google.resumable_media.InvalidResponse`
:param error: A caught exception from the ``google-resumable-media``
library.
:raises: :class:`~google.cloud.exceptions.GoogleCloudError` corresponding
to the failed status code
"""
response = error.response
error_message = str(error)
message = u"{method} {url}: {error}".format(
method=response.request.method, url=response.request.url, error=error_message
)
> raise exceptions.from_http_status(response.status_code, message, response=response)
E google.api_core.exceptions.Forbidden: 403 POST https://storage.googleapis.com/upload/storage/v1/b/new_1585772487875/o?uploadType=multipart: ('Request failed with status code', 403, 'Expected one of', <HTTPStatus.OK: 200>)

@IlyaFaerIlyaFaer changed the title fix(storage): use OrderedDict() while encoding POST policyfix(storage): use OrderedDict while encoding POST policyApr 1, 2020
@frankynfrankyn added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Apr 1, 2020
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 1, 2020

@frankynfrankyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving, one side note. Thanks @IlyaFaer

"expiration": policy_expires.isoformat() + "Z",
}.items()
)
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a little concerned about imposing order in signing code given it's only used for conformance tests.

However, it doesn't change the outcome of expectation and will work as expected as a user.

@frankyn
frankyn merged commit df560e1 into googleapis:masterApr 1, 2020
@IlyaFaer
IlyaFaer deleted the post_policies_flaky_fix branch April 1, 2020 21:40
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
* use OrderedDict() while encoding POST policy
* fix(storage): use OrderedDict() while encoding POST policy
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
* use OrderedDict() while encoding POST policy
* fix(storage): use OrderedDict() while encoding POST policy
@release-pleaserelease-pleaseBot mentioned this pull request Jan 12, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: storageIssues related to the googleapis/python-storage API.cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@IlyaFaer@frankyn@googlebot@yoshi-kokoro