Destination BigQuery: Accept Dataset ID field prefixed by Project ID #8383

koji-m · 2021-12-01T14:14:35Z

What

Closes: #1192

How

In the BigQuery destination connector's Dataset ID field, we could accept both syntax:

project-id:dataset_id
dataset_id

Make it error, if project-id in the Dataset ID field doesn't match the value in the Project ID field.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here
After the new connector version is published, connector version bumped in the seed directory as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

This change is

mkhokh-33 · 2021-12-20T18:13:16Z

Hi, @koji-m could you please update the branch and resolve conflicts on PR, tnx

koji-m · 2021-12-22T14:37:41Z

Hi, @mkhokh-33
I've updated the branch and resolved conflicts. And integration-test passed at local.
Please review.

mkhokh-33 · 2021-12-22T14:54:04Z

...ation-bigquery/src/main/java/io/airbyte/integrations/destination/bigquery/BigQueryUtils.java

@@ -46,6 +49,7 @@

  private static final Logger LOGGER = LoggerFactory.getLogger(BigQueryUtils.class);
  private static final String BIG_QUERY_DATETIME_FORMAT = "yyyy-MM-dd HH:mm:ss.SSSSSS";
+  private static final Pattern datasetIdPattern = Pattern.compile("^(([a-z]([a-z0-9\\-]*[a-z0-9])?):)?([a-zA-Z0-9_]+)$");


it seems to be a constant
pls, rename it to DATASET_ID_PATTERN

mkhokh-33 · 2021-12-22T14:55:14Z

...n-bigquery/src/test/java/io/airbyte/integrations/destination/bigquery/BigQueryUtilsTest.java

+        .put(BigQueryConsts.CONFIG_DATASET_LOCATION, "US");
+  }
+
+  public static Stream<Arguments> validBigQueryIdProvider() {


change method definition to private and move below the public methods

mkhokh-33 · 2021-12-22T14:55:23Z

...n-bigquery/src/test/java/io/airbyte/integrations/destination/bigquery/BigQueryUtilsTest.java

+    assertEquals(expected, actual);
+  }
+
+  public static Stream<Arguments> invalidBigQueryIdProvider() {


change method definition to private and move below the public methods

mkhokh-33 · 2021-12-22T15:48:08Z

...n-bigquery/src/test/java/io/airbyte/integrations/destination/bigquery/BigQueryUtilsTest.java

+
+  public static Stream<Arguments> invalidBigQueryIdProvider() {
+    return Stream.of(
+        Arguments.arguments("my-project", ":my_dataset", "BigQuery Dataset ID format must match '[project-id:]dataset_id': :my_dataset"),


to check regexp pattern for dataset we could also add here some other incorrect datasets:
1."my-project:my-project:my_dataset".(to check that ':' is not allowed inside project name)
2."my-project-:my_dataset". (project name cannot end with a hyphen)
3."my-project:"
3."my-project: "

mkhokh-33 · 2021-12-22T16:05:06Z

...bigquery/src/main/java/io/airbyte/integrations/destination/bigquery/BigQueryDestination.java

@@ -56,7 +56,7 @@ public BigQueryDestination() {
  @Override
  public AirbyteConnectionStatus check(final JsonNode config) {
    try {
-      final String datasetId = config.get(BigQueryConsts.CONFIG_DATASET_ID).asText();
+      final String datasetId = BigQueryUtils.getDatasetId(config);


BigQueryUtils.getDatasetId(config) used in 2 cases

BigQueryDestination.check()

BigQueryDestination.getConsumer() invoke inside BigQueryUploaderFactory.getUploader then BigQueryUtils.getSchema then BigQueryUtils.getDatasetId(config)

Could you pls consider to add integration test to BigQueryDestinationTest to check that:

we can create connection with provided dataset

we can get consumer and write some data

mkhokh-33 · 2021-12-22T16:09:19Z

Hi, @mkhokh-33 I've updated the branch and resolved conflicts. And integration-test passed at local. Please review.

hi @koji-m, thanks for your PR, I left some comments

koji-m · 2021-12-30T11:10:05Z

@mkhokh-33 Thank you for your review. I've fixed based on your review.

mkhokh-33 · 2022-01-04T16:36:16Z

LGTM,
connectors base fails is known issue will be fixed by - #9274
@sherifnada could you have a look please, integration test are passed - #9282

…te into parsing-dataset-names-in-bq

koji-m · 2022-01-06T14:27:04Z

@edgao Thank you for your review. I've fixed based on your review.

edgao

nice! @mkhokh-33 can you take care of releasing this?

edgao · 2022-01-06T16:29:08Z

actually, lemme publish a separate PR first, will comment here once that finishes

edgao · 2022-01-06T18:11:29Z

alright @mkhokh-33 we're good to go here!

mkhokh-33 · 2022-01-10T15:57:06Z

Hi @koji-m ,
could you please update docker images version in following files:

https://github.com/koji-m/airbyte/blob/parsing-dataset-names-in-bq/airbyte-integrations/connectors/destination-bigquery/Dockerfile

LABEL io.airbyte.version=0.6.2

we also need to change version for destination-bigquery-denormalized cause it uses destination-bigquery implementation
2. https://github.com/koji-m/airbyte/blob/parsing-dataset-names-in-bq/airbyte-integrations/connectors/destination-bigquery-denormalized/Dockerfile

LABEL io.airbyte.version=0.2.3

https://github.com/koji-m/airbyte/blob/parsing-dataset-names-in-bq/airbyte-config/init/src/main/resources/seed/destination_definitions.yaml

dockerImage: "airbyte/destination-bigquery:0.6.2"
dockerImage: "airbyte/destination-bigquery-denormalized:0.2.3"

"dockerImageTag": "0.2.3",

https://github.com/koji-m/airbyte/blob/parsing-dataset-names-in-bq/airbyte-config/init/src/main/resources/seed/destination_specs.yaml
-dockerImage: "airbyte/destination-bigquery:0.6.2

dockerImage: "airbyte/destination-bigquery-denormalized:0.2.3"

Thus I could proceed with merge. Thanks

koji-m · 2022-01-16T05:17:56Z

@mkhokh-33 Sorry for the late response. I've updated the docker image version numbers. (with docs)

destination-bigquery: 0.6.3 (master) -> 0.6.4 (this branch)
destination-bigquery-denormalized: 0.2.3 (master) -> 0.2.4 (this branch)

mkhokh-33 · 2022-01-18T12:42:27Z

@mkhokh-33 Sorry for the late response. I've updated the docker image version numbers. (with docs)

destination-bigquery: 0.6.3 (master) -> 0.6.4 (this branch)

destination-bigquery-denormalized: 0.2.3 (master) -> 0.2.4 (this branch)

@koji-m thank you, merged 👍

koji-m added 2 commits December 1, 2021 22:14

add Dataset ID parse method

7f7b220

add BigQuery Destination unit test

69b510d

github-actions bot added the area/connectors Connector related issues label Dec 1, 2021

octavia-squidington-iii added the community label Dec 1, 2021

update change log

5d99d33

github-actions bot added the area/documentation Improvements or additions to documentation label Dec 1, 2021

mkhokh-33 assigned mkhokh-33 and unassigned mkhokh-33 Dec 20, 2021

koji-m added 3 commits December 21, 2021 22:34

Merge branch 'master' into parsing-dataset-names-in-bq

ddd1425

fit to the latest code base

d3d9cda

update change log

f4ced62

mkhokh-33 suggested changes Dec 22, 2021

View reviewed changes

koji-m added 5 commits December 29, 2021 02:54

change var name to const name

a3838eb

change public method to private

af80798

add test cases for testGetDatasetIdFail

4c78cb5

add integration test for dataset-id prefixed with project-id

7171871

Merge branch 'master' into parsing-dataset-names-in-bq

85ab152

mkhokh-33 temporarily deployed to more-secrets January 4, 2022 14:21 Inactive

jrhizor temporarily deployed to more-secrets January 4, 2022 14:43 Inactive

mkhokh-33 temporarily deployed to more-secrets January 4, 2022 14:52 Inactive

mkhokh-33 temporarily deployed to more-secrets January 4, 2022 15:25 Inactive

mkhokh-33 requested a review from sherifnada January 4, 2022 15:43

sherifnada requested a review from edgao January 5, 2022 08:14

karinakuz added connectors/destination/bigquery connectors/destinations-api labels Jan 6, 2022

koji-m added 2 commits January 6, 2022 14:23

add comment to parameterized test provider

686e2c7

Merge branch 'parsing-dataset-names-in-bq' of github.com:koji-m/airby…

d4522f4

…te into parsing-dataset-names-in-bq

edgao approved these changes Jan 6, 2022

View reviewed changes

karinakuz added connectors/destinations-warehouse and removed connectors/destinations-api labels Jan 13, 2022

koji-m added 3 commits January 16, 2022 04:51

update docker image versions

6c26a63

update docker image versions again

25bf817

Merge branch 'master' into parsing-dataset-names-in-bq

5eefebe

mkhokh-33 temporarily deployed to more-secrets January 17, 2022 11:32 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 17, 2022 11:34 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 17, 2022 11:35 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 17, 2022 12:15 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 17, 2022 12:21 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 17, 2022 13:43 Inactive

mkhokh-33 temporarily deployed to more-secrets January 18, 2022 08:38 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 18, 2022 08:59 Inactive

octavia-squidington-iii temporarily deployed to more-secrets January 18, 2022 09:10 Inactive

mkhokh-33 temporarily deployed to more-secrets January 18, 2022 09:41 Inactive

mkhokh-33 merged commit 3f9cbec into airbytehq:master Jan 18, 2022

octavia-squidington-iii mentioned this pull request Jan 20, 2022

Bump Airbyte version from 0.35.5-alpha to 0.35.6-alpha #9630

Merged

edgao mentioned this pull request Mar 11, 2022

🐛 Destination BigQuery: Handle embedded project ID in dataset ID during normalization #11077

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Destination BigQuery: Accept Dataset ID field prefixed by Project ID #8383

Destination BigQuery: Accept Dataset ID field prefixed by Project ID #8383

koji-m commented Dec 1, 2021 •

edited by davinchia

Loading

mkhokh-33 commented Dec 20, 2021

koji-m commented Dec 22, 2021

mkhokh-33 Dec 22, 2021

mkhokh-33 Dec 22, 2021

mkhokh-33 Dec 22, 2021

mkhokh-33 Dec 22, 2021

mkhokh-33 Dec 22, 2021

mkhokh-33 commented Dec 22, 2021

koji-m commented Dec 30, 2021

mkhokh-33 commented Jan 4, 2022

koji-m commented Jan 6, 2022

edgao left a comment

edgao commented Jan 6, 2022

edgao commented Jan 6, 2022

mkhokh-33 commented Jan 10, 2022 •

edited

Loading

koji-m commented Jan 16, 2022

mkhokh-33 commented Jan 18, 2022

Destination BigQuery: Accept Dataset ID field prefixed by Project ID #8383

Destination BigQuery: Accept Dataset ID field prefixed by Project ID #8383

Conversation

koji-m commented Dec 1, 2021 • edited by davinchia Loading

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

mkhokh-33 commented Dec 20, 2021

koji-m commented Dec 22, 2021

mkhokh-33 Dec 22, 2021

Choose a reason for hiding this comment

mkhokh-33 Dec 22, 2021

Choose a reason for hiding this comment

mkhokh-33 Dec 22, 2021

Choose a reason for hiding this comment

mkhokh-33 Dec 22, 2021

Choose a reason for hiding this comment

mkhokh-33 Dec 22, 2021

Choose a reason for hiding this comment

mkhokh-33 commented Dec 22, 2021

koji-m commented Dec 30, 2021

mkhokh-33 commented Jan 4, 2022

koji-m commented Jan 6, 2022

edgao left a comment

Choose a reason for hiding this comment

edgao commented Jan 6, 2022

edgao commented Jan 6, 2022

mkhokh-33 commented Jan 10, 2022 • edited Loading

koji-m commented Jan 16, 2022

mkhokh-33 commented Jan 18, 2022

koji-m commented Dec 1, 2021 •

edited by davinchia

Loading

mkhokh-33 commented Jan 10, 2022 •

edited

Loading