Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
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:mastergoogleapis/python-storage:masterfrom
MaxxleLLC:post_policies_flaky_fixMaxxleLLC/python-storage:post_policies_flaky_fixCopy head branch name to clipboard
Apr 1, 2020
Merged

fix(storage): use OrderedDict while encoding POST policy#95
frankyn merged 3 commits into
googleapis:mastergoogleapis/python-storage:masterfrom
MaxxleLLC:post_policies_flaky_fixMaxxleLLC/python-storage:post_policies_flaky_fixCopy head branch name to clipboard

Conversation

@IlyaFaer

@IlyaFaer IlyaFaer commented Apr 1, 2020

Copy link
Copy Markdown

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

@IlyaFaer IlyaFaer added the api: storage Issues related to the googleapis/python-storage API. label Apr 1, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 1, 2020
@IlyaFaer IlyaFaer changed the title use OrderedDict() while encoding POST policy fix(storage): use OrderedDict() while encoding POST policy Apr 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.

Comment thread tests/unit/test_client.py
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 frankyn April 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>)

@IlyaFaer IlyaFaer changed the title fix(storage): use OrderedDict() while encoding POST policy fix(storage): use OrderedDict while encoding POST policy Apr 1, 2020
@frankyn frankyn 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-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 1, 2020

@frankyn frankyn 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:master Apr 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: storage Issues related to the googleapis/python-storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.