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

fix: revise blob.compose query parameters if_generation_match - #454

Merged
cojenco merged 16 commits into
masterfrom
obj-compose-retry
Jun 14, 2021
Merged

fix: revise blob.compose query parameters if_generation_match#454
cojenco merged 16 commits into
masterfrom
obj-compose-retry

Conversation

@cojenco

@cojencocojenco commented Jun 5, 2021

Copy link
Copy Markdown
Contributor

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 review from a teamJune 5, 2021 01:19
@product-auto-labelproduct-auto-labelBot added the api: storage Issues related to the googleapis/python-storage API. label Jun 5, 2021
@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Jun 5, 2021
Comment threadgoogle/cloud/storage/blob.py Outdated
Comment threadgoogle/cloud/storage/blob.py
Comment threadgoogle/cloud/storage/blob.py
Comment threadgoogle/cloud/storage/blob.py Outdated
Comment threadgoogle/cloud/storage/blob.py Outdated
@cojencocojenco added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 7, 2021
@tseaver

Copy link
Copy Markdown
Contributor

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

@google-cla

google-claBot commented Jun 8, 2021

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 8, 2021
@tseaver

Copy link
Copy Markdown
Contributor

@googlebot I consent.

@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 8, 2021
@tseavertseaver removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 8, 2021
@tseavertseaver mentioned this pull request Jun 8, 2021
@tseavertseaver added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 8, 2021
@tseaver

Copy link
Copy Markdown
Contributor

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

@cojenco

Copy link
Copy Markdown
ContributorAuthor

Got it, thanks @tseaver!

@cojenco
cojenco requested a review from andrewsgJune 8, 2021 19:42
@cojenco

Copy link
Copy Markdown
ContributorAuthor

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

@snippet-bot

Copy link
Copy Markdown

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
Copy Markdown
ContributorAuthor

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

Comment threadgoogle/cloud/storage/blob.py
Comment threadgoogle/cloud/storage/blob.py
Comment threadgoogle/cloud/storage/blob.py
Comment threadtests/system/test_system.py
Comment threadtests/system/test_system.py
Comment threadgoogle/cloud/storage/blob.py Outdated
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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done. I like this phrasing a lot, thanks!

Comment threadgoogle/cloud/storage/blob.py
Comment threadgoogle/cloud/storage/blob.py
Comment threadgoogle/cloud/storage/blob.py Outdated
@cojencocojenco removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 11, 2021
@cojenco
cojenco requested a review from andrewsgJune 14, 2021 16:10

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

Thanks!

@cojenco
cojenco merged commit 70d19e7 into masterJun 14, 2021
@cojenco
cojenco deleted the obj-compose-retry branch June 14, 2021 22:31
cojenco added a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…leapis#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 pull request Oct 13, 2021
…leapis#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>
@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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change blob.compose to fit API behavior

3 participants

@cojenco@tseaver@andrewsg