You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "pabloem (via GitHub)" <gi...@apache.org> on 2023/01/26 22:10:49 UTC
[GitHub] [beam] pabloem opened a new pull request, #25195: Fixing a table existence verifier for BQ
pabloem opened a new pull request, #25195:
URL: https://github.com/apache/beam/pull/25195
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on a diff in pull request #25195: Fixing a table existence verifier for BQ
Posted by "pabloem (via GitHub)" <gi...@apache.org>.
pabloem commented on code in PR #25195:
URL: https://github.com/apache/beam/pull/25195#discussion_r1210743421
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryHelpers.java:
##########
@@ -593,10 +593,15 @@ static String getDatasetLocation(
static void verifyTablePresence(DatasetService datasetService, TableReference table) {
try {
- datasetService.getTable(table);
+ Table fetchedTable = datasetService.getTable(table);
+ if (fetchedTable == null) {
+ throw new IOException("Table does not exist.");
Review Comment:
ah note that this exception is caught again on line 603 and formatted properly. Why do we do this? because `datasetService.getTable` can also throw an exception so we deal with both cases there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ahmedabu98 commented on pull request #25195: Fixing a table existence verifier for BQ
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on PR #25195:
URL: https://github.com/apache/beam/pull/25195#issuecomment-1406851570
Changes LGTM but what was the edge case that wasn't being caught here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem merged pull request #25195: Fixing a table existence verifier for BQ
Posted by "pabloem (via GitHub)" <gi...@apache.org>.
pabloem merged PR #25195:
URL: https://github.com/apache/beam/pull/25195
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #25195: Fixing a table existence verifier for BQ
Posted by "pabloem (via GitHub)" <gi...@apache.org>.
pabloem commented on PR #25195:
URL: https://github.com/apache/beam/pull/25195#issuecomment-1406902653
I found that we were returning null and not throwing an error for tables that do not exist, and thus the function would seem to confirm that the table existed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #25195: Fixing a table existence verifier for BQ
Posted by "pabloem (via GitHub)" <gi...@apache.org>.
pabloem commented on PR #25195:
URL: https://github.com/apache/beam/pull/25195#issuecomment-1405784289
r: @ahmedabu98
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] liferoad commented on a diff in pull request #25195: Fixing a table existence verifier for BQ
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on code in PR #25195:
URL: https://github.com/apache/beam/pull/25195#discussion_r1210706423
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryHelpers.java:
##########
@@ -593,10 +593,15 @@ static String getDatasetLocation(
static void verifyTablePresence(DatasetService datasetService, TableReference table) {
try {
- datasetService.getTable(table);
+ Table fetchedTable = datasetService.getTable(table);
+ if (fetchedTable == null) {
+ throw new IOException("Table does not exist.");
Review Comment:
can we print the table name as well? or may be add some actionable steps for users to take like create the table first with the right schema?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #25195: Fixing a table existence verifier for BQ
Posted by "pabloem (via GitHub)" <gi...@apache.org>.
pabloem commented on PR #25195:
URL: https://github.com/apache/beam/pull/25195#issuecomment-1405860879
Run Java_GCP_IO_Direct PreCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org