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
Member

@IlyaFaer IlyaFaer commented May 20, 2020

Towards #127

@IlyaFaer
Copy link
Member Author

@IlyaFaer IlyaFaer commented May 20, 2020

@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
@IlyaFaer IlyaFaer marked this pull request as ready for review May 20, 2020
Copy link
Contributor

@andrewsg andrewsg left a comment

Thanks for identifying this proactively!

@andrewsg
Copy link
Contributor

@andrewsg andrewsg commented May 20, 2020

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

@gcf-merge-on-green
Copy link

@gcf-merge-on-green gcf-merge-on-green bot commented May 22, 2020

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

@gcf-merge-on-green gcf-merge-on-green bot commented May 22, 2020

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

@gcf-merge-on-green gcf-merge-on-green bot commented May 22, 2020

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.

@gcf-merge-on-green gcf-merge-on-green bot merged commit 76dd9ac into googleapis:master May 22, 2020
3 checks passed
@IlyaFaer IlyaFaer deleted the restrict_generation_match_args branch May 22, 2020
cojenco added a commit to cojenco/python-storage that referenced this issue Oct 13, 2021
cojenco added a commit to cojenco/python-storage that referenced this issue Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants