Skip to content

Conversation

@cojenco
Copy link
Contributor

@cojenco cojenco commented Jun 15, 2022

Fixes #814 🦕

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/python-storage API. labels Jun 15, 2022
@cojenco cojenco marked this pull request as ready for review June 15, 2022 21:19
@cojenco cojenco requested review from a team as code owners June 15, 2022 21:19
@cojenco cojenco added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 23, 2022
if not isinstance(blob_name, str):
blob_name = blob.name
if blob.generation:
generation = blob.generation
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still uncertain whether we should change behavior in this way, though I do agree the current behavior is bad. If we do make this change, can we skip the "take the name from the blob and recreate it" runaround and just run delete on the blob objects directly? This seems unnecessarily convoluted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand your concerns. Unfortunately, with the current setup, it will be difficult to bypass "take the name from the blob...". Reasons being (1) params blobs can be a list of str blob_name OR blob instances, (2) bucket.delete_blob() only takes str blob_name whereas blob.delete() requires a blob instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed, I'm wondering if adding a flag to the method will better ensure backwards compatibility, but I'm also concerned that it will make this even more convoluted >"<

@cojenco
Copy link
Contributor Author

cojenco commented Aug 11, 2022

superseded by #840

@cojenco cojenco closed this Aug 11, 2022
@cojenco cojenco deleted the 814generation branch November 16, 2023 22:07
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. do not merge Indicates a pull request not ready for merge, due to either quality or timing. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bucket.delete_blobs drops 'generation'

3 participants