chore(deps): gcs-resumable-upload migration#1736
Conversation
| import {createURI, ErrorWithCode, upload} from '../src/gcs-resumable-upload'; | ||
|
|
||
| const bucketName = process.env.BUCKET_NAME || 'gcs-resumable-upload-test'; | ||
| const filePath = path.join(__dirname, '../../system-test/data/20MB.zip'); |
There was a problem hiding this comment.
If we can, let's remove this binary in favor of generating a temp file. See:
- https://nodejs.org/api/os.html#ostmpdir
- https://nodejs.org/api/fs.html#fspromisesmkdtempprefix-options
If we want a 20MB file of random bytes:
crypto.randomBytes(1024 * 1024 * 20)
This way we don't have to check in a test blob into the repo.
There was a problem hiding this comment.
Good call. I would agree as well, I'm not a huge fan of committing blobs.
|
After this is merged, we should consider a refactor:
|
| tmp.setGracefulCleanup(); | ||
| const tmpFileContents = crypto.randomBytes(1024 * 1024 * 20); | ||
| const filePath = path.join(os.tmpdir(), '20MB.zip'); | ||
| fs.createWriteStream(filePath).write(tmpFileContents); |
There was a problem hiding this comment.
This stream should be closed to avoid open handle issues (https://nodejs.org/api/fs.html#filehandlecreatereadstreamoptions)
Perhaps write stream would be easier? https://nodejs.org/api/fs.html#fswritesyncfd-buffer-offset-length-position
|
gcs-resumable-upload v4.0.0 has been released - should we include it in this PR? |
…:googleapis/nodejs-storage into shaffeeullah/gcsresumableuploadmigration
|
late review here: |
No description provided.