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

fix: remove dependency on google-cloud-bigquery (cyclic dep) #1295

Merged
merged 8 commits into from Sep 9, 2021

Conversation

stephaniewang526
Copy link
Member

@stephaniewang526 stephaniewang526 commented Sep 7, 2021

Fixes #1249

@stephaniewang526 stephaniewang526 requested review from as code owners Sep 7, 2021
@stephaniewang526 stephaniewang526 requested a review from tswast Sep 7, 2021
@google-cla google-cla bot added the cla: yes label Sep 7, 2021
@product-auto-label product-auto-label bot added the api: bigquerystorage label Sep 7, 2021
@generated-files-bot
Copy link

@generated-files-bot generated-files-bot bot commented Sep 7, 2021

Warning: This pull request is touching the following templated files:

  • .kokoro/dependencies.sh
  • samples/install-without-bom/pom.xml
  • samples/snapshot/pom.xml
  • samples/snippets/pom.xml

@stephaniewang526 stephaniewang526 requested a review from yirutang Sep 8, 2021
@@ -39,27 +37,31 @@ public static void runWriteToDefaultStream()
String projectId = "MY_PROJECT_ID";
String datasetName = "MY_DATASET_NAME";
String tableName = "MY_TABLE_NAME";

writeToDefaultStream(projectId, datasetName, tableName);
TableFieldSchema strField =
Copy link
Contributor

@yirutang yirutang Sep 8, 2021

Choose a reason for hiding this comment

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

I would suggest keep the sample the way it is. Move BQV2ToBQStorageConverter.java into sample as a lib, and the sample would still do a GetTable(), use BQV2ToBQStorageConverter to convert the schema and then use JsonWriter. It doesn't need to block this PR.

Copy link
Member Author

@stephaniewang526 stephaniewang526 Sep 8, 2021

Choose a reason for hiding this comment

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

Sure. Will make this change in a separate PR.

tswast
tswast approved these changes Sep 8, 2021
Copy link

@tswast tswast left a comment

I imagine you'll want a helper in the google-cloud-bigquery client for converting to the BQ Storage TableSchema once the v1 protos are available. That should be possible without a circular dependency.

@@ -47,11 +47,11 @@ function completenessCheck() {
# This is stripped from the output as it is not present in the flattened pom.
# Only dependencies with 'compile' or 'runtime' scope are included from original dependency list.
msg "Generating dependency list using original pom..."
mvn dependency:list -f pom.xml -DincludeScope=runtime -Dsort=true | grep '\[INFO] .*:.*:.*:.*:.*' | sed -e 's/ --.*//' >.org-list.txt
mvn dependency:list -f pom.xml -DincludeScope=runtime -DexcludeArtifactIds=gson,commons-codec,commons-logging,opencensus-contrib-http-util,httpclient,httpcore -Dsort=true | grep '\[INFO] .*:.*:.*:.*:.*' | sed -e 's/ --.*//' >.org-list.txt
Copy link

@tswast tswast Sep 8, 2021

Choose a reason for hiding this comment

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

Where is this exclusion list coming from? A template?

Copy link
Member Author

@stephaniewang526 stephaniewang526 Sep 8, 2021

Choose a reason for hiding this comment

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

Yes indeed. It comes from synthtool. There is a bug in maven-dependency-plugin which is causing this issue: https://issues.apache.org/jira/browse/MDEP-737
So we temporarily add this exclusion as a workaround.

@stephaniewang526 stephaniewang526 added automerge kokoro:force-run kokoro:run and removed kokoro:force-run kokoro:run labels Sep 8, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Sep 8, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 7ac47de into master Sep 9, 2021
16 checks passed
@gcf-merge-on-green gcf-merge-on-green bot deleted the refactor branch Sep 9, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge label Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquerystorage cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants