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/04/30 16:29:05 UTC

[GitHub] [beam] Firlej opened a new pull request, #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

Firlej opened a new pull request, #17517:
URL: https://github.com/apache/beam/pull/17517

   `WriteToBigQuery` pipeline returns `errors` when trying to insert rows that do not match the BigQuery table schema. `errors` is a dictionary that cointains one `FailedRows` key. `FailedRows` is a list of tuples where each tuple has two elements: BigQuery table name and the row that didn't match the schema.
   
   This can be verified by running the `BigQueryIO deadletter pattern` https://beam.apache.org/documentation/patterns/bigqueryio/
   
   Using the template approach I can print the failed rows in a pipeline. When running the job, logger simultaneously prints out the reason why the rows were invalid. 
   
   The reason (for why the row is invalid) should also be included in the tuple in addition to the BigQuery table and the raw row. This way next pipeline could eg. insert the invalid rows into a different BigQuery table with a schema.
   ```python
   error_schema = (
       {
           'fields': [
               {'name': 'timestamp', 'type': 'TIMESTAMP', 'mode': 'REQUIRED'},
               {'name': 'table', 'type': 'STRING', 'mode': 'REQUIRED'},
               {'name': 'reason', 'type': 'STRING', 'mode': 'NULLABLE'},
               {'name': 'row_json', 'type': 'STRING', 'mode': 'REQUIRED'},
           ]
       }
   )
   ```
   
   The whole pipeline implementation could look something like this
   ```python
   with beam.Pipeline(options=pipeline_options) as p:
   
       errors = (
           p
           | "Read from Pub/Sub subscription" >> beam.io.gcp.pubsub.ReadFromPubSub(
               subscription=known_args.input_subscription,
               timestamp_attribute=None)
           | "UTF-8 bytes to string" >> beam.Map(lambda msg: msg.decode("utf-8"))
           | "Parse JSON messages" >> beam.Map(json.loads)
           | "WriteToBigQuery" >> beam.io.WriteToBigQuery(
               known_args.output_table,
               schema=schema,
               create_disposition=beam.io.BigQueryDisposition.CREATE_NEVER,
               write_disposition=beam.io.BigQueryDisposition.WRITE_APPEND,
               ignore_unknown_columns=False,
               insert_retry_strategy=RetryStrategy.RETRY_ON_TRANSIENT_ERROR
           )
       )
       
       result = (
           errors["FailedRows"]
           | 'ParseErrors' >> beam.Map(lambda err: {
                   "timestamp": time.time_ns() / 1000000000,
                   "table": err[0],
                   "reason": None, # TODO to be replaced with `err[2]`
                   "row_json": json.dumps(err[1]),
               })
           | "WriteErrorsToBigQuery" >> beam.io.WriteToBigQuery(
               known_args.output_table + "_error_records",
               schema=error_schema,
               create_disposition=beam.io.BigQueryDisposition.CREATE_IF_NEEDED,
               write_disposition=beam.io.BigQueryDisposition.WRITE_APPEND,
               ignore_unknown_columns=False,
               insert_retry_strategy=RetryStrategy.RETRY_ON_TRANSIENT_ERROR
           )
       )
   ```
   
   During my reasearch I found a couple of alternate solutions, but they are more complex than they need to be. Thats why I explored the beam source code and found the solution to be an easy and simple change.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, 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] asf-ci commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   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] pabloem commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   my bad. I am adding a fix here: https://github.com/apache/beam/pull/17584
   
   On Mon, May 9, 2022 at 10:01 AM Brian Hulette ***@***.***>
   wrote:
   
   > It looks like we didn't get a green run on the Python PostComit before
   > merging, the new test is failing at HEAD. I filed BEAM-14447
   > <https://issues.apache.org/jira/browse/BEAM-14447> to track the failure.
   > Could you take a look @Firlej <https://github.com/Firlej>?
   >
   > If its not quick to diagnose and fix, we might just rollback this PR to
   > preserve test signal. It's easy enough to roll it forward with a fix once
   > we figure it out.
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/beam/pull/17517#issuecomment-1121349920>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AAJ5Z3FLUUGJLOMTKMB4ZXLVJFAF7ANCNFSM5UYRZW5A>
   > .
   > You are receiving this because you modified the open/close state.Message
   > ID: ***@***.***>
   >
   


-- 
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 #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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


##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -1780,8 +1781,9 @@ def _flush_batch(self, destination):
     return [
         pvalue.TaggedOutput(
             BigQueryWriteFn.FAILED_ROWS,
-            GlobalWindows.windowed_value((destination, row)))
-        for row in failed_rows
+            GlobalWindows.windowed_value((destination, row, row_errors)))

Review Comment:
   I wonder if there's a way to do this in a backwards compatible way?
   
   if not, could you add a mention of this change in `CHANGES.md`, so that users will be aware of the difference in 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] Firlej commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   R: @jrmccluskey 


-- 
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 pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   Run PythonLint 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] jrmccluskey commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   R: @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] Firlej commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   @TheNeuralBit I updated `CHANGES.md` with a note regarding the changes implemented 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] asf-ci commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   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] pabloem merged pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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


-- 
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 #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   I'll agree with Brian, and say that we can accept this inl. It's onlybreaking one particular usage, and it adds to the functionality.
   
   LGTM.


-- 
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 pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   Thanks @Firlej! I triggers a CI check that should exercise this new test.
   
   This looks good, I guess my only concern is this could be a breaking change for existing users, e.g. if they're unpacking the current result `destination, row = value`. I think that's OK as long as we document this as a breaking change in CHANGES.md, but I think @chamikaramj or @pabloem should make that call.


-- 
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 pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   It looks like we didn't get a green run on the Python PostComit before merging, the new test is failing at HEAD. I filed BEAM-14447 to track the failure. Could you take a look @Firlej?
   
   If its not quick to diagnose and fix, we might just rollback this PR to preserve test signal. It's easy enough to roll it forward with a fix once we figure it out.


-- 
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 pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   Run Python 3.7 PostCommit


-- 
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 pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   @pabloem could you take a look at this Python BigQueryIO PR?
   
   @Firlej would it be possible to add a test exercising this?


-- 
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 #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   ah makes sense. I'll fix that as well.


-- 
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] codecov[bot] commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #17517:
URL: https://github.com/apache/beam/pull/17517#issuecomment-1114017968

   # [Codecov](https://codecov.io/gh/apache/beam/pull/17517?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17517](https://codecov.io/gh/apache/beam/pull/17517?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (996f2ac) into [master](https://codecov.io/gh/apache/beam/commit/83a8855e5997e0311e6274c03bcb38f94efbf8ef?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (83a8855) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17517      +/-   ##
   ==========================================
   - Coverage   73.83%   73.83%   -0.01%     
   ==========================================
     Files         690      690              
     Lines       90830    90843      +13     
   ==========================================
   + Hits        67065    67074       +9     
   - Misses      22556    22560       +4     
     Partials     1209     1209              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.65% <100.00%> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/17517?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5) | `63.76% <100.00%> (ø)` | |
   | [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
   | [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `93.03% <0.00%> (-0.39%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.90% <0.00%> (-0.16%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (ø)` | |
   | [.../python/apache\_beam/ml/inference/sklearn\_loader.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL3NrbGVhcm5fbG9hZGVyLnB5) | | |
   | [...thon/apache\_beam/ml/inference/sklearn\_inference.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL3NrbGVhcm5faW5mZXJlbmNlLnB5) | `92.68% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/transforms/util.py](https://codecov.io/gh/apache/beam/pull/17517/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy91dGlsLnB5) | `96.06% <0.00%> (+0.08%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17517?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/17517?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [83a8855...996f2ac](https://codecov.io/gh/apache/beam/pull/17517?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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 #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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


##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -1780,8 +1781,9 @@ def _flush_batch(self, destination):
     return [
         pvalue.TaggedOutput(
             BigQueryWriteFn.FAILED_ROWS,
-            GlobalWindows.windowed_value((destination, row)))
-        for row in failed_rows
+            GlobalWindows.windowed_value((destination, row, row_errors)))

Review Comment:
   oh I see that you already updated it. Thanks. Makes sense!



-- 
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 #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   I haven't forgotten about this. It's in my queue : )


-- 
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] Firlej commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   @TheNeuralBit i've implemented a test based on the other tests in the `sdks/python/apache_beam/io/gcp/bigquery_write_it_test.py` file.


-- 
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] Firlej commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   I'm checking it RN


-- 
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] Firlej commented on pull request #17517: [BEAM-14383] Improve "FailedRows" errors returned by beam.io.WriteToBigQuery

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

   @pabloem I see you refactored and simplified the test. I guess it was needlessly complicated.
   
   In hindsight I shouldn't have used a literal string. I don't understand why it threw a `KeyError` Exception though.
   
   Additionally I see [this line](https://github.com/apache/beam/blob/178441dcf0a1e03611c46bdd12ab27907289e310/sdks/python/apache_beam/io/gcp/bigquery_test.py#L1542) throws an error according to [this testReport](https://ci-beam.apache.org/job/beam_PostCommit_Python37/5198/testReport/junit/apache_beam.io.gcp.bigquery_write_it_test/BigQueryWriteIntegrationTests/test_big_query_write_insert_errors_reporting/) posted by @TheNeuralBit in [BEAM-14447](https://issues.apache.org/jira/browse/BEAM-14447)


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