[BEAM-48] Implement BigQueryIO.Read as Source #190
Conversation
peihe
force-pushed
the
custom-bigquery-source
branch
3 times, most recently
from
daf2b28 to
9d0451d
Compare
| .setDestinationTable(destinationTable) | ||
| .setFlattenResults(flattenResults) | ||
| .setPriority("BATCH") | ||
| .setWriteDisposition("WRITE_EMPTY"); |
There was a problem hiding this comment.
I believe you need to also set the location of the temp table, or else BigQuery will use a default. This caused trouble for for tables in EU -- GoogleCloudPlatform/DataflowJavaSDK#86
There was a problem hiding this comment.
done
creates the dataset with the query location
peihe
force-pushed
the
custom-bigquery-source
branch
3 times, most recently
from
47ca135 to
f82a8ca
Compare
|
rebased |
| @Nullable | ||
| Boolean flattenResults; | ||
| @Nullable Boolean flattenResults; | ||
| @Nullable BigQueryServices testBigQueryServices; |
|
Looks pretty good to me, though I think that more functionality could be moved into the Services. I'll be back Monday for further review. Thanks Pei! |
| */ | ||
| Table getTable(String projectId, String datasetId, String tableId) | ||
| throws InterruptedException, IOException; | ||
|
|
There was a problem hiding this comment.
Javadoc. Also, can you please clearly state what invariants and failure behavior these functions have (possibly, in the impl classes instead).
peihe
force-pushed
the
custom-bigquery-source
branch
from
7d9264f to
1ff3427
Compare
peihe
force-pushed
the
custom-bigquery-source
branch
2 times, most recently
from
c508048 to
1f4d00f
Compare
|
Addressed all comments. PTAL |
peihe
force-pushed
the
custom-bigquery-source
branch
from
1f4d00f to
6c104b5
Compare
| if (!dirMatch.isEmpty()) { | ||
| Collection<String> extractFiles = factory.match( | ||
| factory.resolve(extractDestinationDir, "*")); | ||
| new GcsUtilFactory().create(options).remove(extractFiles); |
There was a problem hiding this comment.
directory removal does not work on GCS -- should explicitly enumerate all the files and delete them.
| /** | ||
| * Returns a randomUUID string without {@code '-'}. | ||
| */ | ||
| private static String randomUUIDString() { |
There was a problem hiding this comment.
how about "randomJobId"
And then in the comments explain why you remove the '-'
There was a problem hiding this comment.
But, it is also used in dataset id and temp directory.
Added comments.
|
Does not compile. |
peihe
force-pushed
the
custom-bigquery-source
branch
from
934f610 to
6b49956
Compare
|
PTAL |
|
LGTM and merged |
|
Next steps:
|
2 participants
No description provided.