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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: use util.makeWritableStream #276

Conversation

stephenplusplus
Copy link
Contributor

@stephenplusplus stephenplusplus commented Nov 3, 2014

The BQ PR abstracted the async fetch/setting of a writable stream. This makes Storage use it, and adds tests for the function itself, util.makeWritableStream.

@ryanseys
Copy link
Contributor

ryanseys commented Nov 4, 2014

I noticed that you've been dropping in dup.end() in various places, in this PR and others. I'm assuming it was just forgot. Can a test be included to ensure that this is done in this code i.e. that the stream is closed when it should be. Sorry in advance if this test is already available somewhere or its unnecessary/incorrect to do that?

@stephenplusplus
Copy link
Contributor Author

The codebase has just been following my knowledge lifecycle of streams. I called .end explicitly in the BigQuery PR where util.makeWritableStream was introduced, and now here, where makeWritableStream is used. It's just a matter of making the last change consistent. Everything is tested 👍

@ryanseys
Copy link
Contributor

ryanseys commented Nov 4, 2014

Okey dokey! I figured the codebase just evolves as your understanding of streams does. Additionally, my silly questions should fade out as I grasp more myself. :)

@stephenplusplus
Copy link
Contributor Author

Streams are mind blowing 💨 👴

connection: {
createAuthorizedReq: function(request, callback) {
var contentType = request.headers['Content-Type'];
boundary = contentType.match(/boundary="([^"]*)/)[1];

This comment was marked as spam.

@ryanseys
Copy link
Contributor

ryanseys commented Nov 5, 2014

Looks good to me other than minor suggestions noted.

@stephenplusplus stephenplusplus force-pushed the spp--storage-use-makeWritableStream branch from b66fbdf to 8d67abd Compare November 6, 2014 14:38
@stephenplusplus
Copy link
Contributor Author

Thanks for the review! RegEx documented. Going to merge to make room for the other PRs.

stephenplusplus added a commit that referenced this pull request Nov 6, 2014
…tableStream

storage: use util.makeWritableStream
@stephenplusplus stephenplusplus merged commit 204b3e1 into googleapis:master Nov 6, 2014
sofisl pushed a commit that referenced this pull request Sep 16, 2022
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
sofisl pushed a commit that referenced this pull request Oct 5, 2022
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
sofisl pushed a commit that referenced this pull request Oct 5, 2022
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
sofisl pushed a commit that referenced this pull request Oct 8, 2022
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
sofisl pushed a commit that referenced this pull request Oct 11, 2022
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
sofisl pushed a commit that referenced this pull request Oct 12, 2022
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
sofisl pushed a commit that referenced this pull request Oct 13, 2022
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [jsdoc-fresh](https://github.com/googleapis/jsdoc-fresh) | [`^1.0.1` -> `^2.0.0`](https://renovatebot.com/diffs/npm/jsdoc-fresh/1.1.1/2.0.0) | [![age](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/compatibility-slim/1.1.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/jsdoc-fresh/2.0.0/confidence-slim/1.1.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>googleapis/jsdoc-fresh</summary>

### [`v2.0.0`](https://github.com/googleapis/jsdoc-fresh/blob/HEAD/CHANGELOG.md#&#8203;200-httpsgithubcomgoogleapisjsdoc-freshcomparev111v200-2022-05-18)

[Compare Source](https://github.com/googleapis/jsdoc-fresh/compare/v1.1.1...v2.0.0)

##### ⚠ BREAKING CHANGES

-   update library to use Node 12 ([#&#8203;108](https://github.com/googleapis/jsdoc-fresh/issues/108))

##### Build System

-   update library to use Node 12 ([#&#8203;108](https://github.com/googleapis/jsdoc-fresh/issues/108)) ([e61c223](https://github.com/googleapis/jsdoc-fresh/commit/e61c2238db8900e339e5fe7fb8aea09642290182))

##### [1.1.1](https://www.github.com/googleapis/jsdoc-fresh/compare/v1.1.0...v1.1.1) (2021-08-11)

##### Bug Fixes

-   **build:** migrate to using main branch ([#&#8203;83](https://www.github.com/googleapis/jsdoc-fresh/issues/83)) ([9474adb](https://www.github.com/googleapis/jsdoc-fresh/commit/9474adbf0d559d319ff207397ba2be6b557999ac))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-monitoring-dashboards).
sofisl pushed a commit that referenced this pull request Nov 11, 2022
This PR was generated using Autosynth. 🌈

Synth log will be available here:
https://source.cloud.google.com/results/invocations/663471aa-c039-4d53-b978-92e058276a4e/targets

- [ ] To automatically regenerate this PR, check this box.

Source-Link: googleapis/synthtool@15013ef
sofisl pushed a commit that referenced this pull request Nov 17, 2022
sofisl pushed a commit that referenced this pull request Jan 26, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants