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

use gcs-resumable-upload module #833

Conversation

stephenplusplus
Copy link
Contributor

Fixes #825

This removes the overhead of supporting resumable uploads in gcloud and uses the (way better tested) gcs-resumable-upload.

I extracted it not just for that reason, but because by itself, it's a pretty useful module that someone could very easily want without wanting to pull down all of gcloud.

@stephenplusplus stephenplusplus added the api: storage Issues related to the Cloud Storage API. label Aug 28, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 28, 2015
@jgeewax
Copy link
Contributor

jgeewax commented Aug 30, 2015

WRT gcs-resumable-upload (https://github.com/stephenplusplus/gcs-resumable-upload), can you explain a bit more about the need for the configstore file?

My understanding of the resumable upload API was that we can ask GCS "what's the last byte position you got?" and it'll tell us "I have up to byte X" and then we can go "Cool, here's byte X+1 through byte X+1+chunksize" and keep going.

If that API is correct, I think I'm missing where the configstore settings come into the picture. If I have the API wrong, then I need a full refresher course :)

@stephenplusplus
Copy link
Contributor Author

Haha, we need to store the unique URI. As far as I know, you can't ask the API for "Where I left off for this file?", you have to ask, "Where did I leave off for ** this crazy weird unique ID ** upload?"

@callmehiphop
Copy link
Contributor

We use ConfigStore to cache the first chunk of an upload, then if the upload is interrupted and resumed, we check that the user is not attempting to upload a different file by comparing the two initial chunks. - https://github.com/stephenplusplus/gcs-resumable-upload/blob/master/index.js#L135

We do this in the current code base as well - https://github.com/GoogleCloudPlatform/gcloud-node/blob/master/lib/storage/file.js#L1453

@@ -1314,273 +1309,35 @@ File.prototype.makePublic = function(callback) {
};

/**
* `startResumableUpload_` uses the Resumable Upload API: http://goo.gl/jb0e9D.

This comment was marked as spam.

@callmehiphop
Copy link
Contributor

@stephenplusplus actually I think you can https://cloud.google.com/storage/docs/json_api/v1/how-tos/upload#resume-upload

edit: nvm! saw you meant for the returned url not the via file name

@jgeewax
Copy link
Contributor

jgeewax commented Aug 31, 2015

I think those docs say that you should pass the session URI and ask it how many bytes were uploaded in the session. However the session URI is the exact thing that @stephenplusplus is talking about saving in config store to persist across the life of the process doing the uploading -- considering the chances that a failed upload would also correspond to a dead process.

In this case, I'm 👍 on storing the session URI for use across processes.

callmehiphop added a commit that referenced this pull request Aug 31, 2015
…le-upload-mod

use gcs-resumable-upload module
@callmehiphop callmehiphop merged commit 60889c1 into googleapis:master Aug 31, 2015
sofisl pushed a commit that referenced this pull request Jan 17, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [webpack](https://github.com/webpack/webpack) | devDependencies | major | [`^4.41.6` -> `^5.0.0`](https://renovatebot.com/diffs/npm/webpack/4.44.2/5.1.0) |

---

### Release Notes

<details>
<summary>webpack/webpack</summary>

### [`v5.1.0`](https://github.com/webpack/webpack/releases/v5.1.0)

[Compare Source](https://github.com/webpack/webpack/compare/v5.0.0...v5.1.0)

### Features

-   expose `webpack` property from `Compiler`
-   expose `cleverMerge`, `EntryOptionPlugin`, `DynamicEntryPlugin`

### Bugfixes

-   missing `require("..").xxx` in try-catch produces a warning instead of an error now
-   handle reexports in concatenated modules correctly when they are side-effect-free
-   fix incorrect deprecation message for ModuleTemplate.hooks.hash

### [`v5.0.0`](https://github.com/webpack/webpack/releases/v5.0.0)

[Compare Source](https://github.com/webpack/webpack/compare/v4.44.2...v5.0.0)

[Announcement and changelog](https://webpack.js.org/blog/2020-10-10-webpack-5-release/)

</details>

---

### Renovate configuration

:date: **Schedule**: "after 9am and before 3pm" (UTC).

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

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

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

---

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

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-vision).
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 Cloud 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.

4 participants