feat: enable bytes read tracking #1074

AVaksman · 2020-02-13T16:58:14Z

Design: Changes to file.createWriteStream()

Introduce a ProgressStream class that extends from native Trasform
- on every chunk emits a 'progress' event with total calculated bytesRead value.
add an instance of ProgressStream to the pipe.
If options.onUploadProgress is provided by user add a 'progress' listener that passes the total
bytesRead value to options.onUploadProgress.
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #956

src/file.ts

system-test/storage.ts

src/file.ts

stephenplusplus · 2020-02-23T13:37:13Z

@AVaksman I forgot about this: https://github.com/googleapis/gcs-resumable-upload/blob/4cec8f74a408ea8dc03e3395555189fa6b09add0/src/index.ts#L451

Looks like we could just forward that event to the user (during resumable uploads, anyway)

AVaksman · 2020-02-24T15:21:12Z

@AVaksman I forgot about this: https://github.com/googleapis/gcs-resumable-upload/blob/4cec8f74a408ea8dc03e3395555189fa6b09add0/src/index.ts#L451

Looks like we could just forward that event to the user (during resumable uploads, anyway)

That's good!
Should we worry about the simple uploads as well (for consistency)?

stephenplusplus · 2020-02-24T19:32:28Z

Should we worry about the simple uploads as well (for consistency)?

Yes, I suppose we should. What do you think?

…s set

AVaksman · 2020-03-04T19:50:44Z

Blocked by

PR feat: add progress events nodejs-common#540 to merge
release chore: release 3.0.0 nodejs-common#542
deprecate Node 8

stephenplusplus · 2020-03-16T16:06:03Z

src/file.ts

@@ -1698,7 +1701,7 @@ class File extends ServiceObject<File> {
      pumpify([
        gzip ? zlib.createGzip() : through(),
        validateStream,
-        fileWriteStream,
+        fileWriteStream.on('progress', evt => stream.emit('progress', evt)),


Can you put this event handler assignment on its own line? Just makes it easier to see what the code does line-by-line.

stephenplusplus

Just a subjective suggestion, but LGTM.

enables bytes read tracking from simple upload

AVaksman · 2020-04-02T13:48:57Z

@bcoe
Now that @google-common v3 is released and progress events from simple upload are available (#540) (1834059) all system tests are passing.
However this PR should not be merged until Node 8 is depricated.

codecov · 2020-04-27T13:57:03Z

Codecov Report

Merging #1074 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1074   +/-   ##
=======================================
  Coverage   99.16%   99.16%           
=======================================
  Files          12       12           
  Lines       11474    11474           
  Branches      597      597           
=======================================
  Hits        11378    11378           
  Misses         95       95           
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 195cd46...195cd46. Read the comment docs.

bcoe

I think this feature is really slick, if @stephenplusplus has given his 👍 I say ship it.

gajus · 2020-06-22T18:22:51Z

There does not seem to be a way to track progress using the upload method.

gajus · 2020-06-22T18:48:19Z

I was wrong. onUploadProgress is supported, but not documented. Seems like a documentation bug.

AVaksman added 2 commits February 13, 2020 11:55

feat: enable bytesrRead tracking on uploads

818f150

refactor: only attach listener if flagged by user

72dbea6

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 13, 2020

AVaksman changed the title ~~Enable bytes read tracking~~ feat: enable bytes read tracking Feb 13, 2020

lint: lint

00aea08

AVaksman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 13, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 13, 2020

stephenplusplus suggested changes Feb 13, 2020

View reviewed changes

src/file.ts Outdated Show resolved Hide resolved

src/file.ts Outdated Show resolved Hide resolved

system-test/storage.ts Outdated Show resolved Hide resolved

src/file.ts Outdated Show resolved Hide resolved

AVaksman mentioned this pull request Mar 2, 2020

feat: add progress events googleapis/nodejs-common#540

Merged

3 tasks

AVaksman and others added 5 commits March 2, 2020 14:22

refactor: forward progress event from resumable or simple upload to user

edf63fa

lint: lint

9131ec5

fix: add listener on a condition

a19deee

Merge branch 'master' into enable_bytesRead_tracking

32c1637

Merge branch 'master' into enable_bytesRead_tracking

039a768

AVaksman added the status: blocked Resolving the issue is dependent on other work. label Mar 2, 2020

AVaksman and others added 4 commits March 4, 2020 14:44

refactor: only register progress listener if onUploadProgress param i…

f063ae9

…s set

feat: add onUploadProgress to file.save()

f5496a1

Merge branch 'master' into enable_bytesRead_tracking

1116127

lint: lint

52dcc37

AVaksman and others added 2 commits March 11, 2020 10:32

Merge branch 'master' into enable_bytesRead_tracking

aa0fcdd

Merge branch 'master' into enable_bytesRead_tracking

04d9c15

stephenplusplus reviewed Mar 16, 2020

View reviewed changes

stephenplusplus approved these changes Mar 16, 2020

View reviewed changes

AVaksman and others added 3 commits March 16, 2020 13:56

Merge branch 'master' into enable_bytesRead_tracking

b74521d

Merge branch 'master' into enable_bytesRead_tracking

2c515c6

refactor: readability

e55cf34

AVaksman and others added 2 commits April 2, 2020 09:23

fix(deps): update dependency @google-cloud/common to v3

84c08a1

enables bytes read tracking from simple upload

Merge branch 'master' into enable_bytesRead_tracking

f960bcb

stephenplusplus approved these changes Apr 2, 2020

View reviewed changes

stephenplusplus added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 2, 2020

AVaksman added 2 commits April 7, 2020 10:37

Merge branch 'master' into enable_bytesRead_tracking

f6a9c1e

Merge branch 'master' into enable_bytesRead_tracking

ed711f7

AVaksman removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. status: blocked Resolving the issue is dependent on other work. labels Apr 20, 2020

Merge branch 'master' into enable_bytesRead_tracking

c80bf75

AVaksman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 27, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 27, 2020

Merge branch 'master' into enable_bytesRead_tracking

5fc5975

bcoe approved these changes Apr 27, 2020

View reviewed changes

AVaksman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 27, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 27, 2020

AVaksman and others added 4 commits April 27, 2020 17:37

test: typo

ae665f9

Merge branch 'master' into enable_bytesRead_tracking

38841f7

Merge branch 'master' into enable_bytesRead_tracking

67bacf2

Merge branch 'master' into enable_bytesRead_tracking

2f9055f

AVaksman requested a review from a team as a code owner May 7, 2020 03:56

stephenplusplus added 2 commits May 12, 2020 07:57

Merge branch 'master' into enable_bytesRead_tracking

512352d

Merge branch 'master' into enable_bytesRead_tracking

66a76e0

stephenplusplus added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 12, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 12, 2020

Merge branch 'master' into enable_bytesRead_tracking

195cd46

stephenplusplus merged commit 0776a04 into googleapis:master May 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable bytes read tracking #1074

feat: enable bytes read tracking #1074

AVaksman commented Feb 13, 2020 •

edited

Loading

stephenplusplus commented Feb 23, 2020

AVaksman commented Feb 24, 2020

stephenplusplus commented Feb 24, 2020

AVaksman commented Mar 4, 2020 •

edited

Loading

stephenplusplus Mar 16, 2020

stephenplusplus left a comment

AVaksman commented Apr 2, 2020 •

edited

Loading

codecov bot commented Apr 27, 2020 •

edited

Loading

bcoe left a comment

gajus commented Jun 22, 2020

gajus commented Jun 22, 2020

feat: enable bytes read tracking #1074

feat: enable bytes read tracking #1074

Conversation

AVaksman commented Feb 13, 2020 • edited Loading

stephenplusplus commented Feb 23, 2020

AVaksman commented Feb 24, 2020

stephenplusplus commented Feb 24, 2020

AVaksman commented Mar 4, 2020 • edited Loading

stephenplusplus Mar 16, 2020

Choose a reason for hiding this comment

stephenplusplus left a comment

Choose a reason for hiding this comment

AVaksman commented Apr 2, 2020 • edited Loading

codecov bot commented Apr 27, 2020 • edited Loading

Codecov Report

bcoe left a comment

Choose a reason for hiding this comment

gajus commented Jun 22, 2020

gajus commented Jun 22, 2020

AVaksman commented Feb 13, 2020 •

edited

Loading

AVaksman commented Mar 4, 2020 •

edited

Loading

AVaksman commented Apr 2, 2020 •

edited

Loading

codecov bot commented Apr 27, 2020 •

edited

Loading