Skip to content
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: add Bucket.reload() and Bucket.update() wrappers to restrict generation match args #153

Merged

Conversation

IlyaFaer
Copy link

Towards #127

@IlyaFaer IlyaFaer added api: storage Issues related to the googleapis/python-storage API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels May 20, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 20, 2020
@IlyaFaer
Copy link
Author

@andrewsg, @frankyn, I've taken one more self review and found the thing, that got no our attention yet. We have only two if_metageneration* args in Bucket.reload() and Bucket.update() methods according to the design, and all four if_*generation* args in Blob.reload() and Blob.update() methods.

All of the above methods are inherited from _PropertyMixin class. I've added all four args into its methods, but that means user can pass if_generation_match arg into Bucket.reload(), which is not correct. More than that these args will be shown in docs, like they are okay to be passed:

Безымянный

I'm proposing to add wrapper methods into Bucket class to restrict if_generation* args, and keep only if_metageneration* ones.

@IlyaFaer IlyaFaer requested review from frankyn and andrewsg May 20, 2020 08:46
@IlyaFaer IlyaFaer marked this pull request as ready for review May 20, 2020 08:46
Copy link
Contributor

@andrewsg andrewsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying this proactively!

@andrewsg
Copy link
Contributor

LGTM but I'll pass this by Frank before merging just in case I'm missing something.

@frankyn frankyn added the automerge Merge the pull request once unit tests and other checks pass. label May 21, 2020
@gcf-merge-on-green
Copy link
Contributor

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

1 similar comment
@gcf-merge-on-green
Copy link
Contributor

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@frankyn frankyn added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 22, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 22, 2020
@gcf-merge-on-green gcf-merge-on-green bot merged commit 76dd9ac into googleapis:master May 22, 2020
@IlyaFaer IlyaFaer deleted the restrict_generation_match_args branch May 22, 2020 06:48
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
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. automerge Merge the pull request once unit tests and other checks pass. 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.

None yet

5 participants