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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: revise blob.compose query parameters if_generation_match #454

Merged
merged 16 commits into from Jun 14, 2021

Conversation

@cojenco
Copy link
Contributor

@cojenco cojenco commented Jun 5, 2021

This changes if_generation_match, if_metageneration_match, if_source_generation_match for blob.compose() to fit API behavior

  • Add pending deprecation warning in cases where if_generation_match and if_metageneration_match is passed in as a list of long. Add conditional handling for backwards compatibility.
  • Update if_generation_match to be correctly included within the API request query parameters if it is type long. This makes the operation conditional on whether the composed object's current generation matches the given value.
  • Add if_source_generation_match as an optional parameter that serves as sourceObjects preconditions. This is sent with the API request body. When provided, the composition only performs if the generation of the source object that would be used matches this value.
  • Revise docstring, example and tests

Fixes #453馃

@cojenco cojenco requested a review from Jun 5, 2021
@cojenco cojenco requested a review from as a code owner Jun 5, 2021
@google-cla google-cla bot added the cla: yes label Jun 5, 2021
google/cloud/storage/blob.py Outdated Show resolved Hide resolved
google/cloud/storage/blob.py Show resolved Hide resolved
google/cloud/storage/blob.py Show resolved Hide resolved
google/cloud/storage/blob.py Outdated Show resolved Hide resolved
google/cloud/storage/blob.py Outdated Show resolved Hide resolved
tseaver
tseaver approved these changes Jun 7, 2021
@tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 8, 2021

@cojenco I will merge / repair the tests broken by my PRs this week.

@google-cla
Copy link

@google-cla google-cla bot commented Jun 8, 2021

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-cla google-cla bot added cla: no and removed cla: yes labels Jun 8, 2021
@tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 8, 2021

@googlebot I consent.

@google-cla google-cla bot added cla: yes and removed cla: no labels Jun 8, 2021
@tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 8, 2021

@cojenco I put back the do not merge label to allow us to consider whether this might be a breaking change.

@cojenco
Copy link
Contributor Author

@cojenco cojenco commented Jun 8, 2021

Got it, thanks @tseaver!

@cojenco cojenco requested a review from andrewsg Jun 8, 2021
@cojenco
Copy link
Contributor Author

@cojenco cojenco commented Jun 8, 2021

This currently will introduce a breaking change. Revising PR for backwards compatibility.

@snippet-bot
Copy link

@snippet-bot snippet-bot bot commented Jun 10, 2021

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@cojenco
Copy link
Contributor Author

@cojenco cojenco commented Jun 10, 2021

Updated PR to handle backwards compatibility. Ready for review:)

google/cloud/storage/blob.py Outdated Show resolved Hide resolved
google/cloud/storage/blob.py Outdated Show resolved Hide resolved
google/cloud/storage/blob.py Outdated Show resolved Hide resolved
tests/system/test_system.py Outdated Show resolved Hide resolved
Setting to 0 makes the operation succeed only if there are no live
versions of the object.
If a list of long is passed in, makes the operation conditional on whether the
Copy link
Contributor

@andrewsg andrewsg Jun 11, 2021

I would recommend phrasing this something like:

"Note: In a previous version, this argument worked identically to the if_source_generation_match argument. For backwards-compatibility reasons, if a list is passed in, this argument will behave like if_source_generation_match and also issue a DeprecationWarning."

Copy link
Contributor Author

@cojenco cojenco Jun 11, 2021

Done. I like this phrasing a lot, thanks!

google/cloud/storage/blob.py Show resolved Hide resolved
google/cloud/storage/blob.py Show resolved Hide resolved
google/cloud/storage/blob.py Outdated Show resolved Hide resolved
@cojenco cojenco requested a review from andrewsg Jun 14, 2021
Copy link
Contributor

@andrewsg andrewsg left a comment

Thanks!

@cojenco cojenco merged commit 70d19e7 into master Jun 14, 2021
6 checks passed
@cojenco cojenco deleted the obj-compose-retry branch Jun 14, 2021
cojenco added a commit to cojenco/python-storage that referenced this issue Oct 13, 2021
鈥eapis#454)

* revise blob.compose logic to match API usage

* update tests

* update system test

* address comments

* 馃 Updates from OwlBot

* revise logic for backwards compatibility

* add tests

* revise docstring

* fix test

* revise to DeprecationWarning

* address comments and revise docstrings

Co-authored-by: Tres Seaver <tseaver@palladion.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
cojenco added a commit to cojenco/python-storage that referenced this issue Oct 13, 2021
鈥eapis#454)

* revise blob.compose logic to match API usage

* update tests

* update system test

* address comments

* 馃 Updates from OwlBot

* revise logic for backwards compatibility

* add tests

* revise docstring

* fix test

* revise to DeprecationWarning

* address comments and revise docstrings

Co-authored-by: Tres Seaver <tseaver@palladion.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
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.

3 participants