KEMBAR78

feature: add JsonStreamWriter, which is built on top of a StreamWriter and writes Json data by first converting to protobuf messages then calling StreamWriter's append by allenc3 · Pull Request #437 · googleapis/java-bigquerystorage · GitHub

feature: add JsonStreamWriter, which is built on top of a StreamWriter and writes Json data by first converting to protobuf messages then calling StreamWriter's append #437

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.

Sign up for GitHub

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

Jump to bottom

Merged

stephaniewang526 merged 41 commits into googleapis:master from allenc3:JsonStreamWriter

Aug 7, 2020

Contributor

allenc3 commented Jul 21, 2020

No description provided.

google-cla bot added the cla: yes label

allenc3 force-pushed the JsonStreamWriter branch from 760e8f0 to 73c5926 Compare

July 21, 2020 20:23

yirutang reviewed

View reviewed changes

Contributor

yirutang left a comment

Schema update part looks good.

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

Wilson720 suggested changes

View reviewed changes

Wilson720 left a comment

Looks good overall. Have few comments.

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

allenc3 force-pushed the JsonStreamWriter branch 2 times, most recently from 6576179 to cfcb01e Compare

July 22, 2020 22:07

codecov bot commented Jul 22, 2020 •

edited

Loading

Codecov Report

Merging #437 into master will increase coverage by 0.29%.
The diff coverage is 79.08%.

@@             Coverage Diff              @@
##             master     #437      +/-   ##
============================================
+ Coverage     78.00%   78.30%   +0.29%     
- Complexity      624      642      +18     
============================================
  Files            56       58       +2     
  Lines          3551     3687     +136     
  Branches        235      245      +10     
============================================
+ Hits           2770     2887     +117     
- Misses          675      687      +12     
- Partials        106      113       +7

Impacted Files	Coverage Δ	Complexity Δ
.../cloud/bigquery/storage/v1alpha2/DirectWriter.java	`80.64% <ø> (ø)`	`5.00 <0.00> (ø)`
...igquery/storage/v1alpha2/ProtoSchemaConverter.java	`96.22% <ø> (+14.40%)`	`10.00 <0.00> (+1.00)`
...ud/bigquery/storage/v1alpha2/JsonStreamWriter.java	`77.19% <77.19%> (ø)`	`12.00 <12.00> (?)`
.../cloud/bigquery/storage/v1alpha2/StreamWriter.java	`82.22% <84.37%> (+0.69%)`	`35.00 <7.00> (+1.00)`
...query/storage/v1alpha2/OnSchemaUpdateRunnable.java	`85.71% <85.71%> (ø)`	`4.00 <4.00> (?)`

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 5b6cadd...fb9adc3. Read the comment docs.

Wilson720 approved these changes

View reviewed changes

allenc3 force-pushed the JsonStreamWriter branch from 860e79a to f14de23 Compare

July 23, 2020 17:00

yirutang reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...test/java/com/google/cloud/bigquery/storage/v1alpha2/it/ITBigQueryWriteManualClientTest.java Show resolved Hide resolved

allenc3 force-pushed the JsonStreamWriter branch from cb6edeb to ea61eae Compare

July 27, 2020 20:01

yirutang reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Outdated Show resolved Hide resolved

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Outdated Show resolved Hide resolved

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Outdated Show resolved Hide resolved

Wilson720 reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

...storage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/OnSchemaUpdateRunnable.java Show resolved Hide resolved

...storage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/OnSchemaUpdateRunnable.java Outdated Show resolved Hide resolved

allenc3 force-pushed the JsonStreamWriter branch from 7d3da73 to 96007cb Compare

July 29, 2020 17:21

Wilson720 approved these changes

View reviewed changes

Wilson720 left a comment

Looks good overall. Thanks!

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Show resolved Hide resolved

...storage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/OnSchemaUpdateRunnable.java Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

yirutang reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Show resolved Hide resolved

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Outdated Show resolved Hide resolved

...rystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriterTest.java Outdated Show resolved Hide resolved

...storage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/OnSchemaUpdateRunnable.java Show resolved Hide resolved

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...rystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriterTest.java Show resolved Hide resolved

allenc3 force-pushed the JsonStreamWriter branch from b99e348 to 5c092f3 Compare

August 3, 2020 18:24

yirutang reviewed

View reviewed changes

...storage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/OnSchemaUpdateRunnable.java Outdated Show resolved Hide resolved

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...rystorage/src/test/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriterTest.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

Wilson720 reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

allenc3 added 11 commits

August 5, 2020 15:02


          Added JsonStreamWriter

cf3d06c


          Finished basics of JsonStreamWriter

eb69cc9


          Working basic JsonStreamWriter

ce4ca4f


          Attempt to implement schema update

15bee1a


          Progress on update schema

5c79a56


          Finished designing of JsonStreamWriter, waiting for JsonToProtoMessag…

a9e00ae

…e to be approved.


          Update append method to convert json data to proto messages.

7ffd886


          Temp commit, somehow missing protos

779b8d3


          Copied over protos

ee33c71


          Added more tests and finalize JsonStreamWriter design

70d908b

ls

5292e08

allenc3 added 7 commits

August 5, 2020 15:02


          Fixed according to suggestions

d3c72c9


          modified IT test

6fcedc4


          temp commit

ca0988b


          Added e2e schema update test, multi thread append JsonStreamWriterTes…

374a740

…t, and fixed refreshAppend in StreamWriter


          Remove commentted out code

255b99c


          Experimenting on writer schema

36fff14


          Fixed according to PR suggestions; dealth with syncrhonization issues…

… by letting StreamWriter handle sending first writer schema, added an additional lock between append and refreshAppend for StreamWriter

allenc3 force-pushed the JsonStreamWriter branch from 3538902 to 0230815 Compare

August 5, 2020 20:03


          Fix null ptr error in StreamWriter

yirutang reviewed

View reviewed changes

...d-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/StreamWriter.java Outdated Show resolved Hide resolved

yirutang reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved


          Added refreshConnection in JsonStreamWriter to let it handle refreshA…

f7aeb47

…ppend, flush, and setDescriptor.

yirutang reviewed

View reviewed changes

Contributor

yirutang left a comment

Looks good, only minor comments.

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved


          Made JsonStreamWriterOnSchemaUpdateRunnable a private inner class and…

1e6f283

… made refreshConnection not public

yirutang approved these changes

View reviewed changes


          Rerun

fe8576b

allenc3 marked this pull request as ready for review

August 6, 2020 20:04

stephaniewang526 added the kokoro:force-run label

yoshi-kokoro removed the kokoro:force-run label

stephaniewang526 reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

stephaniewang526 reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved

stephaniewang526 reviewed

View reviewed changes

...gquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1alpha2/JsonStreamWriter.java Outdated Show resolved Hide resolved


          remove import *

fb9adc3

stephaniewang526 approved these changes

View reviewed changes

stephaniewang526 added kokoro:force-run automerge labels

yoshi-kokoro removed the kokoro:force-run label

stephaniewang526 merged commit b9e6cda into googleapis:master

allenc3 changed the title ~~feature: Added JsonStreamWriter, which is built on top of a StreamWriter and writes Json data by first converting to protobuf messages then calling StreamWriter's append~~ feature: add JsonStreamWriter, which is built on top of a StreamWriter and writes Json data by first converting to protobuf messages then calling StreamWriter's append

shubhwip pushed a commit to shubhwip/java-bigquerystorage that referenced this pull request


          chore: update cloud-rad buckets (googleapis#437)

d87f96c

doclava v2 => staging
docFX v3 => prod

Source-Author: Emily Ball <emilyball@google.com>
Source-Date: Tue Feb 16 17:02:08 2021 -0800
Source-Repo: googleapis/synthtool
Source-Sha: b416a7befcdbc42de41cf387dcf428f894fb812b
Source-Link: googleapis/synthtool@b416a7b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge cla: yes