🐛 Destination GCS: CMEK is supported for the connector #20351

armsepehr · 2022-12-12T01:54:24Z

What

Describe what the change is solving
This branch may fix the following issue: GCS destination is not supported Customer-Managed Encryption Key and it is mentioned in the readme file for the destination.

The config file has a missing field which is misleading me for several hour, so I have included the field and some sample-files to check the destination commands as well as the integration tests easily.

How

Describe the solution
As it is discussed in the issue there are two main methods to solve these issues: First, rewrite GCS without AmazonS3 base class. Second, may set integrity check only for GCS to false. In this branch, the second solution is selected.

Note that at the current implementation, google-cloud-storage has been connected to the server via amazon s3 classes and they have some difference.

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

The change guarantees that the users can send their data to the gcs bucket which is enable by customer-managed encryption key feature.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index 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

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
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

CLAassistant · 2022-12-12T01:54:29Z

All committers have signed the CLA.

marcosmarxm · 2022-12-12T16:04:42Z

@armsepehr please add the checklist you removed from the template and fill the tasks done.
@armsepehr please sign the CLA.

armsepehr · 2022-12-15T13:55:53Z

@sajarin @octavia-squidington-iv @marcosmarxm

Should I do anything more?

marcosmarxm · 2022-12-15T18:13:38Z

airbyte-integrations/connectors/destination-gcs/sample_files/configured_catalog.json

+{
+  "streams": [
+    {
+      "sync_mode": "full_refresh",
+      "destination_sync_mode": "append",
+      "stream": {
+        "name": "ab-airbyte-testing",
+        "supported_sync_modes": ["full_refresh"],


Why are you adding this file?

This file is necessary to verify the "write" command locally. I see the same file in other connector, so I included in gcs too.

marcosmarxm · 2022-12-15T18:14:31Z

airbyte-integrations/connectors/destination-gcs/sample_secrets/config.json

+  },
+  "format": {


This is not related to this change.

This is not related to this change, but I think the creator missed to include the format in "config.json" file. I waste some time to figure out the sample "config.json" has not the format field which is necessary. It give us some strange error message if you do not include the "format" field. In a word, I think it is better to include "format" field in "config.json" file.

marcosmarxm · 2022-12-15T18:14:41Z

airbyte-integrations/connectors/destination-gcs/sample_files/messages.jsonl

+{"type": "RECORD", "record": {"stream": "ab-airbyte-testing", "data": {"_ab_pk": "my_value", "column2": 221, "column3": "2021-01-01T20:10:22", "column4": 1.214, "column5": [1,2,3]}, "emitted_at": 1626172757000}}
+{"type": "RECORD", "record": {"stream": "ab-airbyte-testing", "data": {"_ab_pk": "my_value2", "column2": 222, "column3": "2021-01-02T22:10:22", "column5": [1,2,null]}, "emitted_at": 1626172757000}}


Same for this file.

As I said before, this file was included in other connectors and I think it's necessary to verify the connector locally. In my opinion, it is better to include in the project same as other connector.

marcosmarxm · 2022-12-15T18:15:07Z

/test connector=connectors/destination-bigquery

🕑 connectors/destination-bigquery https://github.com/airbytehq/airbyte/actions/runs/3706802785
✅ connectors/destination-bigquery https://github.com/airbytehq/airbyte/actions/runs/3706802785
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         189     48    75%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1441    629    56%

Build Passed

Test summary info:

All Passed

marcosmarxm · 2022-12-20T19:52:55Z

Hello 👋:skin-tone-2: and thank you for your contribution!

Airbyte has instituted a code freeze between 19 and 30 December, to make sure there are no disruptions during the holidays.
Because of this, reviewing and merging your contribution may take longer than usual.
We apologize for the delay, but we want everyone to have a quiet and happy holiday.

If you have any questions or need further clarification, please don't hesitate to ping via Slack.

grishick · 2023-01-20T16:53:53Z

/test connector=connectors/destination-gcs

🕑 connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/3969626147
❌ connectors/destination-gcs https://github.com/airbytehq/airbyte/actions/runs/3969626147
🐛 https://gradle.com/s/os26adizlklgg

Build Failed

Test summary info:

Could not find result summary

grishick · 2023-01-21T00:51:33Z

Rebased, bumped version, and running tests here: #21682

Community PR #20351: Support CMEK or the GCS destination connector (#21682)

armsepehr requested a review from a team as a code owner December 12, 2022 01:54

octavia-squidington-iv added area/connectors Connector related issues bounty community connectors/destination/gcs labels Dec 12, 2022

armsepehr force-pushed the 18195-CMEK-Support-for-GCS-Destination-Connector branch from 6de821c to 98b96b2 Compare December 13, 2022 03:07

armsepehr changed the title ~~BUGS: CMEK is supported for the GCS destination connector~~ 🐛 Destination GCS: CMEK is supported for the connector Dec 13, 2022

marcosmarxm requested changes Dec 15, 2022

View reviewed changes

sajarin added internal and removed bounty labels Dec 19, 2022

grishick force-pushed the 18195-CMEK-Support-for-GCS-Destination-Connector branch from 710500a to 215380c Compare January 20, 2023 18:42

grishick mentioned this pull request Jan 20, 2023

Community PR: Support CMEK or the GCS destination connector #21682

Merged

grishick added a commit that referenced this pull request Jan 24, 2023

Community PR: Support CMEK or the GCS destination connector (#21682)

615bce0

Community PR #20351: Support CMEK or the GCS destination connector (#21682)

grishick closed this Jan 24, 2023

grishick force-pushed the 18195-CMEK-Support-for-GCS-Destination-Connector branch from 215380c to b7be7aa Compare January 24, 2023 20:50

octavia-squidington-iv removed the area/connectors Connector related issues label Jan 24, 2023

airbyteio temporarily deployed to more-secrets January 24, 2023 21:33 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 22:18 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 22:39 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 22:51 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 23:04 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 23:08 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 23:20 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 23:22 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 23:23 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 23:35 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 24, 2023 23:41 — with GitHub Actions Inactive

airbyteio temporarily deployed to more-secrets January 25, 2023 00:18 — with GitHub Actions Inactive

armsepehr deleted the 18195-CMEK-Support-for-GCS-Destination-Connector branch January 25, 2023 05:49

armsepehr restored the 18195-CMEK-Support-for-GCS-Destination-Connector branch January 25, 2023 05:50

armsepehr deleted the 18195-CMEK-Support-for-GCS-Destination-Connector branch January 30, 2023 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Destination GCS: CMEK is supported for the connector #20351

🐛 Destination GCS: CMEK is supported for the connector #20351

armsepehr commented Dec 12, 2022 •

edited

Loading

CLAassistant commented Dec 12, 2022 •

edited

Loading

marcosmarxm commented Dec 12, 2022

armsepehr commented Dec 15, 2022 •

edited

Loading

marcosmarxm Dec 15, 2022

armsepehr Dec 15, 2022

marcosmarxm Dec 15, 2022

armsepehr Dec 15, 2022

marcosmarxm Dec 15, 2022

armsepehr Dec 15, 2022

marcosmarxm commented Dec 15, 2022 •

edited by github-actions bot

Loading

marcosmarxm commented Dec 20, 2022

grishick commented Jan 20, 2023 •

edited by github-actions bot

Loading

grishick commented Jan 21, 2023

		{"type": "RECORD", "record": {"stream": "ab-airbyte-testing", "data": {"_ab_pk": "my_value", "column2": 221, "column3": "2021-01-01T20:10:22", "column4": 1.214, "column5": [1,2,3]}, "emitted_at": 1626172757000}}
		{"type": "RECORD", "record": {"stream": "ab-airbyte-testing", "data": {"_ab_pk": "my_value2", "column2": 222, "column3": "2021-01-02T22:10:22", "column5": [1,2,null]}, "emitted_at": 1626172757000}}

🐛 Destination GCS: CMEK is supported for the connector #20351

🐛 Destination GCS: CMEK is supported for the connector #20351

Conversation

armsepehr commented Dec 12, 2022 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

CLAassistant commented Dec 12, 2022 • edited Loading

marcosmarxm commented Dec 12, 2022

armsepehr commented Dec 15, 2022 • edited Loading

marcosmarxm Dec 15, 2022

Choose a reason for hiding this comment

armsepehr Dec 15, 2022

Choose a reason for hiding this comment

marcosmarxm Dec 15, 2022

Choose a reason for hiding this comment

armsepehr Dec 15, 2022

Choose a reason for hiding this comment

marcosmarxm Dec 15, 2022

Choose a reason for hiding this comment

armsepehr Dec 15, 2022

Choose a reason for hiding this comment

marcosmarxm commented Dec 15, 2022 • edited by github-actions bot Loading

Build Passed

marcosmarxm commented Dec 20, 2022

grishick commented Jan 20, 2023 • edited by github-actions bot Loading

Build Failed

grishick commented Jan 21, 2023

armsepehr commented Dec 12, 2022 •

edited

Loading

CLAassistant commented Dec 12, 2022 •

edited

Loading

armsepehr commented Dec 15, 2022 •

edited

Loading

marcosmarxm commented Dec 15, 2022 •

edited by github-actions bot

Loading

grishick commented Jan 20, 2023 •

edited by github-actions bot

Loading