Only list gcs folder blobs in GCSToBQLoadRunnable #270
Conversation
|
|
| @@ -359,7 +360,8 @@ private void startGCSToBQLoadTask() { | |||
| BucketInfo bucketInfo = BucketInfo.of(bucketName); | |||
| bucket = gcs.create(bucketInfo); | |||
| } | |||
| GCSToBQLoadRunnable loadRunnable = new GCSToBQLoadRunnable(getBigQuery(), bucket); | |||
| Storage.BlobListOption folderPrefixOption = Storage.BlobListOption.prefix(folderName); | |||
| GCSToBQLoadRunnable loadRunnable = new GCSToBQLoadRunnable(getBigQuery(), bucket, folderPrefixOption); | |||
There was a problem hiding this comment.
does this work as expected if there is no folder prefix configured?
There was a problem hiding this comment.
I believe so. As the GCS_FOLDER_NAME_DEFAULT on the config is set to an empty string, we are filtering for objects with a path prefixed by an empty string which is the same as no filter at all.
At least that was how I thought it would work, and took the passing integration tests as confirmation :)
|
@zachary-povey your proposed integration test flow sounds good |
Codecov Report
@@ Coverage Diff @@
## master #270 +/- ##
============================================
- Coverage 70.87% 70.73% -0.14%
Complexity 301 301
============================================
Files 32 32
Lines 1538 1541 +3
Branches 164 164
============================================
Hits 1090 1090
- Misses 390 393 +3
Partials 58 58
|
|
Hi guys - i've added the integration test as described- it felt a bit hacky (I don't do much java) so feel free to make suggestions for refactoring. On a different note are you aware of the integration test ever having race conditions? I was having intermittent issues where tables were the tables populated by streaming inserts wouldn't get created but on a re-run the problem would go away... |
Functionality for specifying a GCS folder within the bucket to dump objects into was added here. This PR addresses an issue with this feature I have outlined in #271.
I have passed through the folder prefix as a list option to the
GCSToBQLoadRunnable, and all tests seem to be passing for me with the update. However a new integration test should added to recreate the issue and confirm this change fixes it.However, this new test is not a straight forward addition of data and schema, as detailed in the readme section for adding integration tests. I thought it was best discussed here for how it should be implemented, but I would imagine it working as follows: