[CDAP-20948] BigQuery Delta Replication Plugin DataSet Project ID Fix #259
Conversation
docs/bigquery-cdcTarget.md
Outdated
| @@ -43,7 +43,7 @@ that the BigQuery job will run in. `BigQuery Job User` role on this project must | |||
| account to run the job. If a temporary bucket needs to be created, the bucket will also be created in this project and | |||
| 'GCE Storage Bucket Admin' role on this project must be granted to the specified service account to create buckets. | |||
|
|
|||
| **Dataset Project ID**: Project the destination dataset belongs to. This is only required if the dataset is not | |||
| **Dataset Project ID**: Project Id for the destination dataset belongs to. This is only required if the dataset is not | |||
|
Can we add a test? |
shivamVarCS
force-pushed
the
bugfix/315193699-big-query-delta-replication-fix-panw
branch
from
b1eb578
to
d55f585
Compare
Sure, will follow up next time |
I have modified the existing test cases which verifies the latest changes |
Please edit PR description. Add link to Jira. |
| @@ -193,7 +193,7 @@ public void testGetMaximumExistingSequenceNumberForRetryableFailures() throws Ex | |||
| bqTarget.initialize(deltaTargetContext); | |||
| } finally { | |||
| //verify at least 1 retry happens | |||
| PowerMockito.verifyStatic(BigQueryUtils.class, Mockito.atLeast(2)); | |||
| PowerMockito.verifyStatic(BigQueryUtils.class, Mockito.atLeast(1)); | |||
There was a problem hiding this comment.
why did this test change? It seems unrelated to your changes
There was a problem hiding this comment.
As per the new code changes, we do need to mock it twice, as it only requires 1 retry and if mocked twice. it break test in the second retry.
| BigQueryUtils.getMaximumExistingSequenceNumber(context.getAllTables(), project, conf.getDatasetName(), | ||
| bigQuery, encryptionConfig, MAX_TABLES_PER_QUERY)); | ||
| BigQueryUtils.getMaximumExistingSequenceNumber(context.getAllTables(), conf.getDatasetProject(), | ||
| conf.getDatasetName(), bigQuery, encryptionConfig, MAX_TABLES_PER_QUERY)); |
There was a problem hiding this comment.
This project and conf.getDatasetproject() both are different project Id's.
Earlier it was pointing to project, now we are passing DatasetProjectId.
There was a problem hiding this comment.
I meant to say that we should revert the whitespace change (indent), not code change to point to the right project id
shivamVarCS
changed the title
itsankit-google
changed the title
itsankit-google
deleted the
bugfix/315193699-big-query-delta-replication-fix-panw
branch
Objective : BugFix for BigQuery Delta Plugin: Unable to fetch maximum sequence number from existing tables by using Dataset Project Id.
Jira: Link