Redshift Destination: update spec #12100

VitaliiMaltsev · 2022-04-18T10:44:28Z

What

Updated spec to follow connector release requirements

🚨 User Impact 🚨

none

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.

misteryeo

Added some suggestions and comments.

misteryeo · 2022-04-18T20:18:45Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

@@ -110,10 +110,10 @@
        "maximum": 100,
        "examples": ["10"],
        "description": "Optional. Increase this if syncing tables larger than 100GB. Only relevant for COPY. Files are streamed to S3 in parts. This determines the size of each part, in MBs. As S3 has a limit of 10,000 parts per file, part size affects the table size. This is 10MB by default, resulting in a default limit of 100GB tables. Note, a larger part size will result in larger memory requirements. A rule of thumb is to multiply the part size by 10 to get the memory requirement. Modify this with care.",


Suggested change

"description": "Optional. Increase this if syncing tables larger than 100GB. Only relevant for COPY. Files are streamed to S3 in parts. This determines the size of each part, in MBs. As S3 has a limit of 10,000 parts per file, part size affects the table size. This is 10MB by default, resulting in a default limit of 100GB tables. Note, a larger part size will result in larger memory requirements. A rule of thumb is to multiply the part size by 10 to get the memory requirement. Modify this with care.",

"description": "Increase this if syncing tables larger than 100GB. Only relevant for COPY. Files are streamed to S3 in parts. This determines the size of each part, in MBs. As S3 has a limit of 10,000 parts per file, part size affects the table size. This is 10MB by default, resulting in a default limit of 100GB tables. Note: a larger part size will result in larger memory requirements. A rule of thumb is to multiply the part size by 10 to get the memory requirement. Modify this with care.",

misteryeo · 2022-04-18T20:19:31Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

@@ -95,13 +95,13 @@
      "access_key_id": {
        "type": "string",
        "description": "The Access Key Id granting allow one to access the above S3 staging bucket. Airbyte requires Read and Write permissions to the given bucket.",


Suggested change

"description": "The Access Key Id granting allow one to access the above S3 staging bucket. Airbyte requires Read and Write permissions to the given bucket.",

"description": "This ID grants access to the above S3 staging bucket. Airbyte requires Read and Write permissions to the given bucket.",

misteryeo · 2022-04-18T20:19:41Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

        "type": "string",
        "description": "The directory under the S3 bucket where data will be written. If not provided, then defaults to the root directory.",
        "examples": ["data_sync/test"]
      },
      "s3_bucket_region": {
-        "title": "S3 Bucket Region",
+        "title": "S3 Bucket Region (Optional)",
        "type": "string",
        "default": "",
        "description": "The region of the S3 staging bucket to use if utilising a copy strategy.",


Suggested change

"description": "The region of the S3 staging bucket to use if utilising a copy strategy.",

"description": "The region of the S3 staging bucket to use if utilising a COPY strategy.",

misteryeo · 2022-04-18T20:20:16Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

      },
      "purge_staging_data": {
-        "title": "Purge Staging Files and Tables",
+        "title": "Purge Staging Files and Tables (Optional)",
        "type": "boolean",
        "description": "Whether to delete the staging files from S3 after completing the sync. See the docs for details. Only relevant for COPY. Defaults to true.",


Where are the docs?

"See the docs for details."

misteryeo · 2022-04-20T16:27:51Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

        "type": "string",
        "description": "The name of the staging S3 bucket to use if utilising a COPY strategy. COPY is recommended for production workloads for better speed and scalability. See <a href=\"https://docs.aws.amazon.com/redshift/latest/dg/c_loading-data-best-practices.html\">AWS docs</a> for more details.",
        "examples": ["airbyte.staging"]
      },
      "s3_bucket_path": {
-        "title": "S3 Bucket Path",
+        "title": "S3 Bucket Path (Optional)",
        "type": "string",
        "description": "The directory under the S3 bucket where data will be written. If not provided, then defaults to the root directory.",


Are there no relevant docs to link here?

misteryeo · 2022-04-20T16:28:31Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

        "airbyte_secret": true
      },
      "part_size": {
        "type": "integer",
        "minimum": 10,
        "maximum": 100,
        "examples": ["10"],
-        "description": "Optional. Increase this if syncing tables larger than 100GB. Only relevant for COPY. Files are streamed to S3 in parts. This determines the size of each part, in MBs. As S3 has a limit of 10,000 parts per file, part size affects the table size. This is 10MB by default, resulting in a default limit of 100GB tables. Note, a larger part size will result in larger memory requirements. A rule of thumb is to multiply the part size by 10 to get the memory requirement. Modify this with care.",
-        "title": "Stream Part Size"
+        "description": "Increase this if syncing tables larger than 100GB. Only relevant for COPY. Files are streamed to S3 in parts. This determines the size of each part, in MBs. As S3 has a limit of 10,000 parts per file, part size affects the table size. This is 10MB by default, resulting in a default limit of 100GB tables. Note: a larger part size will result in larger memory requirements. A rule of thumb is to multiply the part size by 10 to get the memory requirement. Modify this with care.",


Are they no docs to link here?

@VitaliiMaltsev?

misteryeo · 2022-04-20T16:28:55Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

        "type": "boolean",
-        "description": "Whether to delete the staging files from S3 after completing the sync. See the docs for details. Only relevant for COPY. Defaults to true.",
+        "description": "Whether to delete the staging files from S3 after completing the sync. See the docs for details. https://docs.airbyte.com/integrations/destinations/redshift#2a-fill-up-s3-info-for-copy-strategy. Only relevant for COPY. Defaults to true.",


We are we including the URL directly and not hyperlinking the text?

misteryeo · 2022-04-20T16:29:25Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

        "type": "string",
        "default": "",
-        "description": "The region of the S3 staging bucket to use if utilising a copy strategy.",
+        "description": "The region of the S3 staging bucket to use if utilising a COPY strategy. See the docs for details. See the docs for details. https://docs.aws.amazon.com/AmazonS3/latest/userguide/creating-bucket.html#:~:text=In-,Region,-%2C%20choose%20the%20AWS",


Why is the URL not hyperlinked? And I think you've put the text in twice for 'see the docs..'?

VitaliiMaltsev · 2022-04-21T12:33:21Z

@misteryeo added changes according to your suggestions

misteryeo

Just had one comment here where I wasn't seeing docs but otherwise LGTM!

misteryeo · 2022-04-26T18:16:46Z

airbyte-integrations/connectors/destination-redshift/src/main/resources/spec.json

        "airbyte_secret": true
      },
      "part_size": {
        "type": "integer",
        "minimum": 10,
        "maximum": 100,
        "examples": ["10"],
-        "description": "Optional. Increase this if syncing tables larger than 100GB. Only relevant for COPY. Files are streamed to S3 in parts. This determines the size of each part, in MBs. As S3 has a limit of 10,000 parts per file, part size affects the table size. This is 10MB by default, resulting in a default limit of 100GB tables. Note, a larger part size will result in larger memory requirements. A rule of thumb is to multiply the part size by 10 to get the memory requirement. Modify this with care.",
-        "title": "Stream Part Size"
+        "description": "Increase this if syncing tables larger than 100GB. Only relevant for COPY. Files are streamed to S3 in parts. This determines the size of each part, in MBs. As S3 has a limit of 10,000 parts per file, part size affects the table size. This is 10MB by default, resulting in a default limit of 100GB tables. Note: a larger part size will result in larger memory requirements. A rule of thumb is to multiply the part size by 10 to get the memory requirement. Modify this with care.",


@VitaliiMaltsev?

VitaliiMaltsev · 2022-04-27T10:58:39Z

Just had one comment here where I wasn't seeing docs but otherwise LGTM!

@misteryeo added link for part_size field

* Redshift Destination: update spec * update spec.json * update links in spec.json * added more links to spec.json | refactoring * updated docs with stadard connector template * added hyperlink to documentation for part_size field

Redshift Destination: update spec

d701e5b

VitaliiMaltsev requested a review from misteryeo April 18, 2022 10:44

github-actions bot added the area/connectors Connector related issues label Apr 18, 2022

misteryeo suggested changes Apr 18, 2022

View reviewed changes

VitaliiMaltsev added 2 commits April 19, 2022 12:46

update spec.json

229f387

update links in spec.json

2989f68

VitaliiMaltsev requested a review from misteryeo April 19, 2022 11:28

misteryeo suggested changes Apr 20, 2022

View reviewed changes

added more links to spec.json | refactoring

32bb749

VitaliiMaltsev requested a review from misteryeo April 21, 2022 12:33

updated docs with stadard connector template

90dc957

github-actions bot added the area/documentation Improvements or additions to documentation label Apr 26, 2022

misteryeo approved these changes Apr 26, 2022

View reviewed changes

added hyperlink to documentation for part_size field

2179693

VitaliiMaltsev merged commit b16e13e into master Apr 27, 2022

VitaliiMaltsev deleted the destination-redshift-update-spec branch April 27, 2022 11:01

octavia-squidington-iii mentioned this pull request Apr 27, 2022

Bump Airbyte version from 0.36.3-alpha to 0.36.4-alpha #12415

Merged

misteryeo mentioned this pull request May 11, 2022

[EPIC] Destination Redshift to GA #11287

Closed

24 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redshift Destination: update spec #12100

Redshift Destination: update spec #12100

VitaliiMaltsev commented Apr 18, 2022

misteryeo left a comment

misteryeo Apr 18, 2022

misteryeo Apr 18, 2022

misteryeo Apr 18, 2022

misteryeo Apr 18, 2022

misteryeo Apr 20, 2022

misteryeo Apr 20, 2022

misteryeo Apr 26, 2022

misteryeo Apr 20, 2022

misteryeo Apr 20, 2022

VitaliiMaltsev commented Apr 21, 2022

misteryeo left a comment

misteryeo Apr 26, 2022

VitaliiMaltsev commented Apr 27, 2022

	"description": "The Access Key Id granting allow one to access the above S3 staging bucket. Airbyte requires Read and Write permissions to the given bucket.",
	"description": "This ID grants access to the above S3 staging bucket. Airbyte requires Read and Write permissions to the given bucket.",

	"description": "The region of the S3 staging bucket to use if utilising a copy strategy.",
	"description": "The region of the S3 staging bucket to use if utilising a COPY strategy.",

Redshift Destination: update spec #12100

Redshift Destination: update spec #12100

Conversation

VitaliiMaltsev commented Apr 18, 2022

What

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

misteryeo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VitaliiMaltsev commented Apr 21, 2022

misteryeo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VitaliiMaltsev commented Apr 27, 2022