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

feat: add ignore_flush parameter to BlobWriter #644

Merged
merged 2 commits into from Nov 4, 2021
Merged

Conversation

andrewsg
Copy link
Contributor

@andrewsg andrewsg commented Nov 2, 2021

Fixes #631 馃

@andrewsg andrewsg requested review from a team as code owners November 2, 2021 01:36
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 2, 2021
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Nov 2, 2021
google/cloud/storage/blob.py Show resolved Hide resolved
google/cloud/storage/fileio.py Outdated Show resolved Hide resolved
google/cloud/storage/fileio.py Outdated Show resolved Hide resolved
@andrewsg
Copy link
Contributor Author

andrewsg commented Nov 3, 2021

PTAL

@@ -3873,23 +3887,38 @@ def open(
raise ValueError(
"encoding, errors and newline arguments are for text mode only"
)
if ignore_flush:
Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewsg Should these instead be if ignore_flush is not None:

if ignore_flush=False is accidentally set for reads, it would cause an AttributeError or such.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point to focus on. I thought about that but decided that since False was the actual desired behavior, we didn't need to guard against it here. I don't think it would cause an AttributeError - is there a place that looks suspicious?

Copy link
Contributor

@cojenco cojenco Nov 3, 2021

Choose a reason for hiding this comment

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

I see what you mean. That's right; it wouldn't cause an error since ignore_flush is not passed in to the BlobReader constructor after all. I misread that.


:type ignore_flush: bool
:param ignore_flush:
Makes flush() do nothing instead of raise an error. flush() without
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to a handwritten doc for FileIO now that I'm looking at it. We could have more thorough explanations written on an individual page, and streamline the docstrings by linking to the page.

@andrewsg andrewsg merged commit af9c9dc into main Nov 4, 2021
@andrewsg andrewsg deleted the fileio-ignore-flush branch November 4, 2021 18:26
gcf-merge-on-green bot pushed a commit that referenced this pull request Nov 17, 2021
馃 I have created a release \*beep\* \*boop\*
---
## [1.43.0](https://www.github.com/googleapis/python-storage/compare/v1.42.3...v1.43.0) (2021-11-15)


### Features

* add ignore_flush parameter to BlobWriter ([#644](https://www.github.com/googleapis/python-storage/issues/644)) ([af9c9dc](https://www.github.com/googleapis/python-storage/commit/af9c9dc83d8582167b74105167af17c9809455de))
* add support for Python 3.10 ([#615](https://www.github.com/googleapis/python-storage/issues/615)) ([f81a2d0](https://www.github.com/googleapis/python-storage/commit/f81a2d054616c1ca1734997a16a8f47f98ab346b))


### Bug Fixes

* raise a ValueError in BucketNotification.create() if a topic name is not set ([#617](https://www.github.com/googleapis/python-storage/issues/617)) ([9dd78df](https://www.github.com/googleapis/python-storage/commit/9dd78df444d21af51af7858e8958b505a26c0b79))


### Documentation

* add contributing and authoring guides under samples/ ([#633](https://www.github.com/googleapis/python-storage/issues/633)) ([420591a](https://www.github.com/googleapis/python-storage/commit/420591a2b71f823dbe80f4a4405d8a514f87e0fb))
* add links to samples and how to guides ([#641](https://www.github.com/googleapis/python-storage/issues/641)) ([49f78b0](https://www.github.com/googleapis/python-storage/commit/49f78b09fed6d9f486639fd0a72542c30a0df084))
* add README to samples subdirectory ([#639](https://www.github.com/googleapis/python-storage/issues/639)) ([58af882](https://www.github.com/googleapis/python-storage/commit/58af882c047c31f59486513c568737082bca6350))
* update samples readme with cli args ([#651](https://www.github.com/googleapis/python-storage/issues/651)) ([75dda81](https://www.github.com/googleapis/python-storage/commit/75dda810e808074d18dfe7915f1403ad01bf2f02))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot stream-write a zipfile
3 participants