Skip to content

Conversation

andrewsg
Copy link
Contributor

@andrewsg andrewsg commented Nov 2, 2021

Fixes #631 🦕

@andrewsg andrewsg requested review from unforced and cojenco November 2, 2021 01:36
@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
@andrewsg andrewsg force-pushed the fileio-ignore-flush branch from dbae485 to 3b9ee31 Compare November 3, 2021 18:54
@andrewsg
Copy link
Contributor Author

andrewsg commented Nov 3, 2021

PTAL

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