migrate to new bq client #201
Conversation
pom.xml
Outdated
| @@ -474,15 +479,24 @@ | |||
| <ignoreSourcePackage> | |||
| <package>org.slf4j</package> | |||
| </ignoreSourcePackage> | |||
| <ignoreSourcePackage> | |||
There was a problem hiding this comment.
Now how useful it is to have missinglink? 🤔
There was a problem hiding this comment.
imo not useful enough
There was a problem hiding this comment.
I will leave this for this PR and we can consider how to move forward in later one.
| @@ -75,25 +71,6 @@ public TableId createStagingTableId(TableId tableId, String location) { | |||
| return randomStagingTableId(tableId, location); | |||
| } | |||
|
|
|||
| @Override | |||
| public BigQueryResult query(QueryRequest queryRequest) { | |||
There was a problem hiding this comment.
Do we want to bring back this method?
There was a problem hiding this comment.
Not unless it's needed?
There was a problem hiding this comment.
Not really. Even query is wrapped in a JobInfo, and QueryRequest should not be used anymore.
|
Note that this is quite a breaking change, but seems no one is using this feature yet. |
contrib/flo-scio_2.12/pom.xml
Outdated
| @@ -16,7 +16,8 @@ | |||
|
|
|||
| <properties> | |||
| <scala.baseVersion>2.12</scala.baseVersion> | |||
| <scala.version>2.12.6</scala.version> | |||
| <scala.version>2.12.8</scala.version> | |||
| <scio.version>0.7.0-beta3</scio.version> | |||
There was a problem hiding this comment.
What is the oldest version of scio we want to support?
There was a problem hiding this comment.
No idea. We never talked about this. There have been some breaking changes in Scio across versions. It would be hard to support them.
There was a problem hiding this comment.
Well, we try to avoid touching the parts of scio that tend to break so that flo can work with a range of scio versions.
There was a problem hiding this comment.
We should probably support at least the latest stable release, i.e. 0.6.1.
There was a problem hiding this comment.
I will take a look, but I remember this PR addresses some incompatibility introduced by scio 0.7 already.
There was a problem hiding this comment.
There are quite many different versions of libs depended by beam2.6 which is depended by scio 0.6.1.
I'm not having a good feeling when it comes to our users of mono repo (which is already on scio 0.7.0-beta3).
There was a problem hiding this comment.
Doing it in a separated PR: #204
| return BigQueryOperation.ofQuery(() -> QueryRequest.of(query)); | ||
| public BigQueryOperation<T> query(String query) { | ||
| final QueryJobConfiguration queryConfig = newBuilder(query) | ||
| // Use standard SQL syntax for queries. |
There was a problem hiding this comment.
Should this be javadoc instead?
There was a problem hiding this comment.
Indeed. I just copied Google's sample code. xD
|
👍 with #204 |
downgrade to scio 0.6.1
Hey, I just made a Pull Request!
Description
Motivation and Context
The current version of bq is not working well with Scio.
Have you tested this? If so, how?
Checklist for PR author(s)
Checklist for PR reviewer(s)