Skip to content

Conversation

@IlyaFaer
Copy link

Towards #38

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 10, 2020
@IlyaFaer IlyaFaer added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Feb 10, 2020
@IlyaFaer
Copy link
Author

IlyaFaer commented Feb 10, 2020

Moved here from googleapis/google-cloud-python#9893

@IlyaFaer IlyaFaer requested review from crwilcox and frankyn February 10, 2020 11:03
@IlyaFaer IlyaFaer marked this pull request as ready for review February 10, 2020 11:03
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.

Two nits but overall LGTM.

@IlyaFaer
Copy link
Author

IlyaFaer commented Feb 12, 2020

Systests failed with (same as in other PRs):

=================================== FAILURES ===================================
__________________ TestStorageBuckets.test_get_set_iam_policy __________________

self = <tests.system.TestStorageBuckets testMethod=test_get_set_iam_policy>

    def test_get_set_iam_policy(self):
        import pytest
        from google.cloud.storage.iam import STORAGE_OBJECT_VIEWER_ROLE
        from google.api_core.exceptions import BadRequest, PreconditionFailed

        bucket_name = "iam-policy" + unique_resource_id("-")
        bucket = retry_429_503(Config.CLIENT.create_bucket)(bucket_name)
        self.case_buckets_to_delete.append(bucket_name)
        self.assertTrue(bucket.exists())

        policy_no_version = bucket.get_iam_policy()
        self.assertEqual(policy_no_version.version, 1)

        policy = bucket.get_iam_policy(requested_policy_version=3)
        self.assertEqual(policy, policy_no_version)

        member = "serviceAccount:{}".format(Config.CLIENT.get_service_account_email())

        BINDING_W_CONDITION = {
            "role": STORAGE_OBJECT_VIEWER_ROLE,
            "members": {member},
            "condition": {
                "title": "always-true",
                "description": "test condition always-true",
                "expression": "true",
            },
        }
        policy.bindings.append(BINDING_W_CONDITION)

        with pytest.raises(
            PreconditionFailed, match="enable uniform bucket-level access"
        ):
            bucket.set_iam_policy(policy)

        bucket.iam_configuration.uniform_bucket_level_access_enabled = True
        bucket.patch()

        policy = bucket.get_iam_policy(requested_policy_version=3)
        policy.bindings.append(BINDING_W_CONDITION)

        with pytest.raises(BadRequest, match="at least 3"):
            bucket.set_iam_policy(policy)

        policy.version = 3
        returned_policy = bucket.set_iam_policy(policy)
        self.assertEqual(returned_policy.version, 3)
        self.assertEqual(returned_policy.bindings, policy.bindings)

        with pytest.raises(
            BadRequest, match="cannot be less than the existing policy version"
        ):
>           bucket.get_iam_policy()
E           Failed: DID NOT RAISE <class 'google.api_core.exceptions.BadRequest'>

tests/system.py:315: Failed

@frankyn
Copy link
Contributor

frankyn commented Feb 12, 2020

@jkwlui is taking a look IIUC, cc: @crwilcox. Thanks @IlyaFaer

@frankyn
Copy link
Contributor

frankyn commented Feb 12, 2020

Looks like they've already fixed it. #52

Merged master and tests are passing.

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.

LGTM

@frankyn frankyn merged commit fe434fc into googleapis:master Feb 12, 2020
@IlyaFaer IlyaFaer deleted the move_bucket_create_into_client branch February 13, 2020 08:21
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…leapis#49)

* refactor(storage): deprecate Bucket.create() and requester_pays

* add_test_to_increase_cover

* resolve conflicts

* resolve conflicts

* add new way of setting requester_pays into doclines

Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…leapis#49)

* refactor(storage): deprecate Bucket.create() and requester_pays

* add_test_to_increase_cover

* resolve conflicts

* resolve conflicts

* add new way of setting requester_pays into doclines

Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants