ahmedabu98 commented on code in PR #25325:
URL: https://github.com/apache/beam/pull/25325#discussion_r1146127690
##########
sdks/python/apache_beam/io/gcp/bigquery_test.py:
##########
@@ -1082,6 +1082,39 @@ def test_insert_all_unretriable_errors(
},
create_disposition='CREATE_NEVER',
method='STREAMING_INSERTS'))
+ self.assertEqual(1, mock_send.call_count)\
+
+ @mock.patch('time.sleep')
+ @mock.patch('google.cloud.bigquery.Client.insert_rows_json')
+ def test_insert_all_table_not_found_errors(
Review Comment:
I don't think this actually tests the changes you're making. It mocks
`insert_rows_json` but doesn't return a 404 error.
An integration test is probably more appropriate for these changes (instead
of writing to a dummy table). And we should have a check at the beginning of
the test to make sure we're running with TestDataflowRunner:
```
if not isinstance(self.test_pipeline.runner, TestDataflowRunner):
self.skipTest("This test will invoke a bundle retry, which needs
TestDataflowRunner")
```
##########
sdks/python/apache_beam/io/gcp/bigquery_test.py:
##########
@@ -1082,6 +1082,39 @@ def test_insert_all_unretriable_errors(
},
create_disposition='CREATE_NEVER',
method='STREAMING_INSERTS'))
+ self.assertEqual(1, mock_send.call_count)\
+
+ @mock.patch('time.sleep')
+ @mock.patch('google.cloud.bigquery.Client.insert_rows_json')
+ def test_insert_all_table_not_found_errors(
Review Comment:
The approach you take in this test looks good (adding to KNOWN_TABLES before
the write), let's do that with a real write to BQ. I suggest moving this test
to `BigQueryStreamingInsertTransformIntegrationTests` and make use of the
output table created in `setUp`:
https://github.com/apache/beam/blob/92abdf636c52cce7c067593000bc846f507b0595/sdks/python/apache_beam/io/gcp/bigquery_test.py#L1606-L1620
##########
sdks/python/apache_beam/io/gcp/bigquery_test.py:
##########
@@ -1082,6 +1082,39 @@ def test_insert_all_unretriable_errors(
},
create_disposition='CREATE_NEVER',
method='STREAMING_INSERTS'))
+ self.assertEqual(1, mock_send.call_count)\
+
+ @mock.patch('time.sleep')
+ @mock.patch('google.cloud.bigquery.Client.insert_rows_json')
+ def test_insert_all_table_not_found_errors(
Review Comment:
P.S. you can use `BigqueryFullResultMatcher` to make sure the data arrived
at the table correctly. Example below:
https://github.com/apache/beam/blob/92abdf636c52cce7c067593000bc846f507b0595/sdks/python/apache_beam/io/gcp/bigquery_test.py#L1657-L1661
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
