You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/06/07 20:34:44 UTC

[GitHub] [beam] vachan-shetty opened a new pull request, #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

vachan-shetty opened a new pull request, #21739:
URL: https://github.com/apache/beam/pull/21739

   This line was erroneously removed in #16231. 
   
   This line triggers the `FAILED_PRECONDITION` checks that are used to figure out if a`splitAtFraction()` call was successful.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Add a link to the appropriate issue in your description, if applicable. This will automatically link the pull request to the issue.
    - [ ] 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/#make-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)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+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] chamikaramj commented on a diff in pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on code in PR #21739:
URL: https://github.com/apache/beam/pull/21739#discussion_r891719608


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java:
##########
@@ -355,6 +356,7 @@ public synchronized BigQueryStorageStreamSource<T> getCurrentSource() {
                       .build(),
                   source.readSession.getTable());
           newResponseIterator = newResponseStream.iterator();
+          newResponseIterator.hasNext();

Review Comment:
   Probably let's add a log line here describing why this call is important. For someone who doesn't understand the details regarding the BQ API, it's just an ignored hasNext call which might be removed in the future cleanups.
   
   Also, does this points to an issue with the API ?



-- 
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] TheNeuralBit commented on a diff in pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on code in PR #21739:
URL: https://github.com/apache/beam/pull/21739#discussion_r892571921


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java:
##########
@@ -355,6 +356,7 @@ public synchronized BigQueryStorageStreamSource<T> getCurrentSource() {
                       .build(),
                   source.readSession.getTable());
           newResponseIterator = newResponseStream.iterator();
+          newResponseIterator.hasNext();

Review Comment:
   Would it be feasible to add a test that exercises this behavior? 



-- 
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] asf-ci commented on pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21739:
URL: https://github.com/apache/beam/pull/21739#issuecomment-1149140122

   Can one of the admins verify this patch?


-- 
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] vachan-shetty commented on pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
vachan-shetty commented on PR #21739:
URL: https://github.com/apache/beam/pull/21739#issuecomment-1149169140

   R: @chamikaramj, @pabloem 


-- 
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] chamikaramj commented on pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on PR #21739:
URL: https://github.com/apache/beam/pull/21739#issuecomment-1149176020

   cc: @TheNeuralBit 


-- 
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] aaltay commented on pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #21739:
URL: https://github.com/apache/beam/pull/21739#issuecomment-1149375076

   /cc @pabloem - Could you please include this in the next release cut?
   
   @vachan-shetty - if this is not merged in the next day or so, could you please create a release blocking github issue to make sure that this change makes to the next release.


-- 
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] chamikaramj commented on pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on PR #21739:
URL: https://github.com/apache/beam/pull/21739#issuecomment-1149449428

   LGTM. Thanks.


-- 
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] asf-ci commented on pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21739:
URL: https://github.com/apache/beam/pull/21739#issuecomment-1149140125

   Can one of the admins verify this patch?


-- 
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] vachan-shetty commented on a diff in pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
vachan-shetty commented on code in PR #21739:
URL: https://github.com/apache/beam/pull/21739#discussion_r891741954


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java:
##########
@@ -355,6 +356,7 @@ public synchronized BigQueryStorageStreamSource<T> getCurrentSource() {
                       .build(),
                   source.readSession.getTable());
           newResponseIterator = newResponseStream.iterator();
+          newResponseIterator.hasNext();

Review Comment:
   Added a comment.
   
   I don't think this indicates an issue with the API per se. We could probably try and read from the stream more explicitly in case that is clearer but the comment also points out why this important now.



-- 
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] chamikaramj merged pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
chamikaramj merged PR #21739:
URL: https://github.com/apache/beam/pull/21739


-- 
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] vachan-shetty commented on pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
vachan-shetty commented on PR #21739:
URL: https://github.com/apache/beam/pull/21739#issuecomment-1149245246

   Run Java 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

[GitHub] [beam] chamikaramj commented on a diff in pull request #21739: Fix for increased FAILED_PRECONDITION errors in BQ Read API.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on code in PR #21739:
URL: https://github.com/apache/beam/pull/21739#discussion_r891719608


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java:
##########
@@ -355,6 +356,7 @@ public synchronized BigQueryStorageStreamSource<T> getCurrentSource() {
                       .build(),
                   source.readSession.getTable());
           newResponseIterator = newResponseStream.iterator();
+          newResponseIterator.hasNext();

Review Comment:
   Probably let's add a comment here describing why this call is important. For someone who doesn't understand the details regarding the BQ API, it's just an ignored hasNext call which might be removed in the future cleanups.
   
   Also, does this points to an issue with the API ?



-- 
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