Skip to content

Bigquery: Fix bug for sourceFormat Bigtable ExternalTableDefinition. #6427

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
pmakani merged 6 commits into from
Oct 21, 2019
Merged

Bigquery: Fix bug for sourceFormat Bigtable ExternalTableDefinition. #6427

pmakani merged 6 commits into from
Oct 21, 2019

Conversation

@pmakani
Copy link

@pmakani pmakani commented Oct 3, 2019 •
edited
Loading

Fixes #6417

@pmakani pmakani requested a review from shollyman October 3, 2019 13:12
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 3, 2019
@athakor athakor added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 3, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 3, 2019
@codecov
Copy link

codecov bot commented Oct 4, 2019 •
edited
Loading

Codecov Report

Merging #6427 into master will decrease coverage by 0.28%.
The diff coverage is 28.57%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6427      +/-   ##
============================================
- Coverage     46.34%   46.06%   -0.29%     
- Complexity    27967    27974       +7     
============================================
  Files          2613     2613              
  Lines        287929   287827     -102     
  Branches      33756    32896     -860     
============================================
- Hits         133443   132577     -866     
+ Misses       144267   144205      -62     
- Partials      10219    11045     +826
Impacted Files Coverage Δ Complexity Δ
...om/google/cloud/bigquery/BigtableColumnFamily.java 96% <ø> (ø) 5 <0> (ø) ⬇️
...java/com/google/cloud/bigquery/BigtableColumn.java 100% <ø> (ø) 5 <0> (ø) ⬇️
...google/cloud/bigquery/ExternalTableDefinition.java 56.84% <0%> (-1.86%) 16 <0> (ø)
...ava/com/google/cloud/bigquery/BigtableOptions.java 59.18% <50%> (-2.52%) 8 <0> (ø)
...ud/devtools/containeranalysis/v1/GrafeasUtils.java 0% <0%> (-90.91%) 0% <0%> (-1%)
.../containeranalysis/v1/ContainerAnalysisClient.java 42.1% <0%> (-8.78%) 15% <0%> (-2%)
...va/com/google/cloud/compute/v1/InstanceClient.java 48.65% <0%> (-6.45%) 147% <0%> (ø)
.../com/google/cloud/compute/v1/RegionDiskClient.java 47.9% <0%> (-5.99%) 43% <0%> (ø)
...ava/com/google/cloud/compute/v1/ProjectClient.java 51.14% <0%> (-5.94%) 55% <0%> (ø)
...ava/com/google/cloud/compute/v1/NetworkClient.java 49.01% <0%> (-5.89%) 39% <0%> (ø)
... and 188 more

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 19eee78...513b4bb. Read the comment docs.

@pmakani pmakani changed the title Bigquery: expose public properties of builder in BigtableOptions. Bigquery: expose public properties and methods for BigtableColumn, BigtableColumnFamily and BigtableOptions. Oct 4, 2019
@pmakani pmakani added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 4, 2019
@pmakani pmakani changed the title Bigquery: expose public properties and methods for BigtableColumn, BigtableColumnFamily and BigtableOptions. Bigquery: Fix bug for Bigtable ExternalTableDefinition. Oct 4, 2019
@pmakani pmakani changed the title Bigquery: Fix bug for Bigtable ExternalTableDefinition. Bigquery: Fix bug for sourceFormat Bigtable ExternalTableDefinition. Oct 4, 2019
@pmakani pmakani mentioned this pull request Oct 4, 2019
Closed
@pmakani pmakani removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 4, 2019
elharo
elharo reviewed Oct 7, 2019
View reviewed changes
...ud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigtableColumn.java
}

static Builder newBuilder() {
public static Builder newBuilder() {
Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

Javadoc

Copy link
Author

@pmakani pmakani Oct 9, 2019

Choose a reason for hiding this comment

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

Done.

...ents/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigtableColumnFamily.java
}

static Builder newBuilder() {
public static Builder newBuilder() {
Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

Javadoc

Copy link
Author

@pmakani pmakani Oct 9, 2019

Choose a reason for hiding this comment

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

Done.

...d-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigtableOptions.java
}

static final class Builder {
public static final class Builder {
Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

Javadoc

Copy link
Author

@pmakani pmakani Oct 9, 2019

Choose a reason for hiding this comment

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

Done.

...d-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigtableOptions.java Outdated
builder.setReadRowkeyAsString(options.getReadRowkeyAsString());
builder.setColumnFamilies(
Lists.transform(options.getColumnFamilies(), BigtableColumnFamily.FROM_PB_FUNCTION));
if (null != options.getIgnoreUnspecifiedColumnFamilies()) {
Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

reverse is more conventional; that is,

options != null

Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

It's unclear why null isn't allowed since that's the only reason to set Boolean instead of boolean

Copy link
Author

@pmakani pmakani Oct 9, 2019

Choose a reason for hiding this comment

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

Done.

...d-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigtableOptions.java Outdated
if (null != options.getIgnoreUnspecifiedColumnFamilies()) {
builder.setIgnoreUnspecifiedColumnFamilies(options.getIgnoreUnspecifiedColumnFamilies());
}
if (null != options.getReadRowkeyAsString()) {
Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

ditto

Copy link
Author

@pmakani pmakani Oct 9, 2019

Choose a reason for hiding this comment

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

Done.

...d-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigtableOptions.java Outdated
if (null != options.getReadRowkeyAsString()) {
builder.setReadRowkeyAsString(options.getReadRowkeyAsString());
}
if (null != options.getColumnFamilies()) {
Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

ditto

Copy link
Author

@pmakani pmakani Oct 9, 2019

Choose a reason for hiding this comment

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

Done.

...s/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/ExternalTableDefinition.java Outdated
/**
* Creates a builder for an ExternalTableDefinition object.
*
* @param sourceUri The fully-qualified URIs that point to your data in Google Cloud. For Google
Copy link
Contributor

@elharo elharo Oct 7, 2019

Choose a reason for hiding this comment

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

The --> the

Copy link
Author

@pmakani pmakani Oct 9, 2019

Choose a reason for hiding this comment

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

Done.

@pmakani pmakani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
@pmakani pmakani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
@athakor athakor added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
@pmakani
Copy link
Author

pmakani commented Oct 14, 2019

@shollyman PTAL.

@pmakani pmakani merged commit c99059c into googleapis:master Oct 21, 2019
@pmakani pmakani deleted the api-bigquery-6417 branch October 21, 2019 05:18
@chingor13 chingor13 mentioned this pull request Oct 30, 2019
Merged
@codyoss codyoss mentioned this pull request Nov 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shollyman shollyman shollyman approved these changes

+1 more reviewer

@elharo elharo elharo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Querying Cloud Bigtable data

6 participants

@pmakani @shollyman @elharo @googlebot @yoshi-kokoro @athakor