-
Notifications
You must be signed in to change notification settings - Fork 85
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
Conversation
Warning: This pull request is touching the following templated files:
|
99f900c
to
60c5462
Compare
.../test/java/com/google/cloud/bigquery/storage/v1beta2/it/ITBigQueryWriteManualClientTest.java
Show resolved
Hide resolved
@@ -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 = |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Fixes #1249