Skip to content

Conversation

IlyaFaer
Copy link

@IlyaFaer IlyaFaer commented Apr 1, 2020

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
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
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
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
Copy link
Author

IlyaFaer commented Apr 1, 2020

@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
Copy link
Contributor

@frankyn frankyn left a comment

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
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 join this conversation on GitHub. Already have an account? Sign in to comment
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