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

feat: public access prevention - #304

Merged
tseaver merged 13 commits into
googleapis:masterfrom
shaffeeullah:publicaccessprevention
Jun 30, 2021
Merged

feat: public access prevention#304
tseaver merged 13 commits into
googleapis:masterfrom
shaffeeullah:publicaccessprevention

Conversation

@shaffeeullah

@shaffeeullahshaffeeullah commented Nov 9, 2020

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes#312 🦕

@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Nov 9, 2020
@product-auto-labelproduct-auto-labelBot added the api: storage Issues related to the googleapis/python-storage API. label Nov 9, 2020

@tritonetritone 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.

Generally looks good-- my biggest question is whether we should use some kind of enum or constant for the values.

Comment threadgoogle/cloud/storage/bucket.py Outdated
Comment threadgoogle/cloud/storage/bucket.py Outdated
Comment threadtests/system/test_system.py Outdated
Comment threadtests/system/test_system.py Outdated
Comment threadgoogle/cloud/storage/bucket.py Outdated
Comment threadgoogle/cloud/storage/bucket.py Outdated
Comment threadgoogle/cloud/storage/bucket.py
Comment threadgoogle/cloud/storage/bucket.py
Comment threadtests/system/test_system.py Outdated
@tseaver

Copy link
Copy Markdown
Contributor

The systest failures seem to indicate that the feature is not working as expected. Do we need to get the CI project added to a whitelist?

@shaffeeullah

Copy link
Copy Markdown
ContributorAuthor

The systest failures seem to indicate that the feature is not working as expected. Do we need to get the CI project added to a whitelist?

The backend feature still has some bugs that are being resolved. The current failure is expected at this time.

Comment threadgoogle/cloud/storage/bucket.py Outdated
Comment threadgoogle/cloud/storage/bucket.py Outdated
Comment threadgoogle/cloud/storage/bucket.py
Comment threadgoogle/cloud/storage/bucket.py
@google-cla

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

1 similar comment
@google-cla

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-clagoogle-claBot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 16, 2020
@google-cla

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

2 similar comments
@google-cla

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@shaffeeullahshaffeeullah added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 23, 2020
@shaffeeullah
shaffeeullah marked this pull request as ready for review December 23, 2020 18:16
@shaffeeullah
shaffeeullah requested a review from a teamDecember 23, 2020 18:16
@shaffeeullah

Copy link
Copy Markdown
ContributorAuthor

Hi @tseaver , can you please sign the CLA? See #304 (comment)

@shaffeeullah

Copy link
Copy Markdown
ContributorAuthor

Hi @tseaver , can you please sign the CLA? See #304 (comment)

@tseaver friendly bump

@googlebot

Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@andrewsg

Copy link
Copy Markdown
Contributor

Tres has signed the CLA and submitted other PRs with the same email address without issue, so I don't know why googlebot is complaining here.

@andrewsg

andrewsg commented Jan 12, 2021

Copy link
Copy Markdown
Contributor

@shaffeeullah This seems to be an issue with Github adding Tres to the contributor list automatically, with different user details than he normally uses based on the git config in his actual computer.

Tres definitely is a CLA signer. I conferred with Frank and he suggested you rebase to remove Tres from the contributor list and force-push. Please message me on chat if you need help wrangling git here. And make a backup of your branch just in case.

@tritonetritone 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.

One minor docs comment, otherwise looks good!

Comment threadgoogle/cloud/storage/bucket.py
@google-cla

google-claBot commented Jun 8, 2021

Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@google-cla

google-claBot commented Jun 8, 2021

Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@shaffeeullah

Copy link
Copy Markdown
ContributorAuthor

Public access prevention rollout has been delayed due to a bug surfaced during Googler preview. I will keep this PR updated as I learn new release timeline details.

@shaffeeullah

Copy link
Copy Markdown
ContributorAuthor

@andrewsg This feature can now be merged and released. Are you able to help me with this?

@tritonetritone removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. status: blocked Resolving the issue is dependent on other work. labels Jun 29, 2021
@tseaver

Copy link
Copy Markdown
Contributor

@shaffeeullah I can help fix the merge, since my PR refactoring the system tests is what broke your new ones.

@google-cla

Copy link
Copy Markdown

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-clagoogle-claBot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Jun 30, 2021
@tseaver

Copy link
Copy Markdown
Contributor

@googlebot I consent.

@google-cla

Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-clagoogle-claBot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jun 30, 2021
@tseavertseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 30, 2021
@google-cla

Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@shaffeeullah

Copy link
Copy Markdown
ContributorAuthor

Thanks, @tseaver !! @andrewsg@tseaver If the tests pass, would it be possible to release this today?

@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 30, 2021
@tseaver

Copy link
Copy Markdown
Contributor

@shaffeeullah Yes, we should be able to make a release. @cojenco, are any of your open PRs at a state where they should be merged before a release?

@tseaver
tseaver merged commit e3e57a9 into googleapis:masterJun 30, 2021
@shaffeeullah
shaffeeullah deleted the publicaccessprevention branch June 30, 2021 15:26
@cojenco

Copy link
Copy Markdown
Contributor

Thanks @tseaver and @shaffeeullah! I'll be merging #480 shortly and trying to squeeze in #484 this morning as well for the release.

@tseaver

Copy link
Copy Markdown
Contributor

@shaffeeullah Release is in progress: PR #485

cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
@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.externalThis issue is blocked on a bug with the actual product.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add support for PublicAccessPrevention

7 participants

@shaffeeullah@tseaver@googlebot@andrewsg@cojenco@tritone@yoshi-kokoro