-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(storage): support optionsRequestedPolicyVersion #9989
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jkwlui
requested review from
busunkim96,
crwilcox,
frankyn and
tseaver
as code owners
December 17, 2019 20:37
googlebot
added
the
cla: yes
This human has signed the Contributor License Agreement.
label
Dec 17, 2019
jkwlui
changed the title
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](https://github.com/googleapis/google-cloud-python/issues) 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)
feat(storage): support optionsRequestedPolicyVersion
Dec 17, 2019
frankyn
reviewed
Dec 17, 2019
frankyn
reviewed
Dec 18, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, I have one nit question.
maintain compatibility with defaultdict remove in place raise KeyError on delete update deprecation for dict-key access and factory methods clean up maintain compatibility - removing duplicate in __setitems__ check for conditions for dict access remove empty binding fix test accessing private var _bindings fix(tests): change version to make existing tests pass tests: add tests for getitem, delitem, setitem on v3 and conditions test policy.bindings property fixlint black sort bindings by role when converting to api repr add deprecation warning for iam factory methods update deprecation message for role methods make Policy#bindings.members a set update policy docs fix docs make docs better fix: Bigtable policy class to use Policy.bindings add from_pb with conditions test add to_pb condition test blacken fix policy __delitem__ add docs on dict access do not modify binding in to_apr_repr
jkwlui
force-pushed
the
iam-proposal3
branch
from
December 19, 2019 21:16
74a32cc
to
583ca42
Compare
jkwlui
force-pushed
the
storage-policy-version
branch
from
December 19, 2019 21:17
1badb64
to
c210d29
Compare
frankyn
approved these changes
Dec 19, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
jkwlui
added
the
do not merge
Indicates a pull request not ready for merge, due to either quality or timing.
label
Dec 19, 2019
crwilcox
suggested changes
Dec 20, 2019
jkwlui
force-pushed
the
iam-proposal3
branch
from
December 21, 2019 00:56
583ca42
to
6c9556d
Compare
jkwlui
added
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Jan 9, 2020
yoshi-kokoro
removed
the
kokoro:force-run
Add this label to force Kokoro to re-run the tests.
label
Jan 9, 2020
@crwilcox fixed the builds. Would you mind taking a look again? |
crwilcox
approved these changes
Jan 13, 2020
This was referenced Jan 29, 2020
Merged
Merged
Merged
[CHANGE ME] Re-generated container to pick up changes in the API or client library generator.
#10256
Closed
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.
do not merge
Indicates a pull request not ready for merge, due to either quality or timing.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires #9869