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

feat: Added schema compatibility check functionality (SchemaCompact.java) #339

Merged

Conversation

allenc3
Copy link
Contributor

@allenc3 allenc3 commented Jun 10, 2020

Finished schema compatibility check class, and added a proto file that is used for testing.

Modified WriterCache.java, DirectWriterTest.java, and WriterCacheTest.java because checking for schema compatibility requires three parameters instead of two, and so the "check" method requires three parameters.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 10, 2020
@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #339 into master will increase coverage by 0.84%.
The diff coverage is 93.70%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #339      +/-   ##
============================================
+ Coverage     75.63%   76.47%   +0.84%     
- Complexity      465      555      +90     
============================================
  Files            54       54              
  Lines          3123     3261     +138     
  Branches        175      196      +21     
============================================
+ Hits           2362     2494     +132     
- Misses          655      658       +3     
- Partials        106      109       +3     
Impacted Files Coverage Δ Complexity Δ
...cloud/bigquery/storage/v1alpha2/SchemaCompact.java 91.19% <93.70%> (+15.00%) 96.00 <91.00> (+89.00)
.../cloud/bigquery/storage/v1alpha2/StreamWriter.java 81.07% <0.00%> (+0.70%) 34.00% <0.00%> (+1.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 8d42ecd...a6e3867. Read the comment docs.

@allenc3 allenc3 force-pushed the SchemaCompactNoAnnotatation branch from 7a36964 to 9bacc48 Compare June 11, 2020 15:57
@allenc3 allenc3 force-pushed the SchemaCompactNoAnnotatation branch from dcb2727 to e8b3e68 Compare June 12, 2020 15:48
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Jun 15, 2020
@allenc3 allenc3 force-pushed the SchemaCompactNoAnnotatation branch from cd4d7ac to 3671801 Compare June 15, 2020 18:40
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jun 15, 2020
Copy link
Contributor

@yirutang yirutang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there, just one more test case requested.

@allenc3 allenc3 force-pushed the SchemaCompactNoAnnotatation branch 2 times, most recently from 3a009dc to 3a03d46 Compare June 16, 2020 17:04
@allenc3 allenc3 force-pushed the SchemaCompactNoAnnotatation branch 2 times, most recently from 892a3eb to c1ceb60 Compare June 17, 2020 19:25
@stephaniewang526 stephaniewang526 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 18, 2020
</differences>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing has changed - please revert this file.


private SchemaCompact(BigQuery bigquery) {
// TODO: Add functionality that allows SchemaCompact to build schemas.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason for us to keep this TODO here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, Yiru asked me to leave that there as that should be a functionality we should add in, but not related to the current PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok sg

@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 18, 2020
@allenc3 allenc3 force-pushed the SchemaCompactNoAnnotatation branch from c15a329 to a6e3867 Compare June 18, 2020 15:35
@allenc3 allenc3 marked this pull request as ready for review June 18, 2020 15:44
@stephaniewang526 stephaniewang526 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 18, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 18, 2020
@stephaniewang526 stephaniewang526 merged commit bc2d8cc into googleapis:master Jun 18, 2020
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants