feat: add support for Hive partitioning options when creating external tables#236
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Codecov Report
@@ Coverage Diff @@
## master #236 +/- ##
=========================================
Coverage ? 77.53%
Complexity ? 1151
=========================================
Files ? 75
Lines ? 6089
Branches ? 654
=========================================
Hits ? 4721
Misses ? 1019
Partials ? 349
Continue to review full report at Codecov.
|
| */ | ||
| @SuppressWarnings("unchecked") | ||
| @Nullable | ||
| public abstract HivePartitioningOptions getHivePartitioningOptions(); |
There was a problem hiding this comment.
this breaks binary compatibility
There was a problem hiding this comment.
Thanks @stephaniewang526 for the review. I will update the patch and send again.
There was a problem hiding this comment.
@stephaniewang526, @pmakani I updated the patch and I wonder if the tests would automatically run?
There was a problem hiding this comment.
@stephaniewang526, thanks for triggering the tests. It was my bad with the previous patch - I missed a line that I fixed locally related to the test failures. Could you please trigger the tests again?
There was a problem hiding this comment.
Thanks @stephaniewang526. All tests passed now.
|
In general, when you make a commit please be sure to:
we don't want to do this because this renders our client unstable and would break customer's existing implementation. I suggest to try either implement without abstract and add default implementation or encapsulate with an inner method. |
| public abstract Builder setSchema(Schema schema); | ||
|
|
||
| /** Sets the table Hive partitioning options. */ | ||
| public abstract Builder setHivePartitioningOptions(HivePartitioningOptions hivePartitioningOptions); |
There was a problem hiding this comment.
this breaks binary compatibility
🤖 I have created a release \*beep\* \*boop\* --- ## [1.110.0](https://www.github.com/googleapis/java-bigquery/compare/v1.109.0...v1.110.0) (2020-03-20) ### Features * add support for Hive partitioning options when creating external tables ([#235](https://www.github.com/googleapis/java-bigquery/issues/235)) ([#236](https://www.github.com/googleapis/java-bigquery/issues/236)) ([ccc2bb3](https://www.github.com/googleapis/java-bigquery/commit/ccc2bb3de28a36e3791d5441c8bdea2333877ee8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #235 ☕️