Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🎉 New Destination: DynamoDB #5561

Merged
merged 10 commits into from
Sep 2, 2021
Merged

🎉 New Destination: DynamoDB #5561

merged 10 commits into from
Sep 2, 2021

Conversation

jinnig
Copy link
Contributor

@jinnig jinnig commented Aug 21, 2021 •
edited
Loading

What

Added AWS DynamoDB as the destination.
https://aws.amazon.com/dynamodb/

Recommended reading order

  1. DynamodbDestination.java
  2. DynamodbChecker.java
  3. DynamodbConsumer.java
  4. DynamodbWriter.java
  5. DynamodbDestinationAcceptanceTest.java

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

  • Community member? 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
    • docs/SUMMARY.md
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions
  • Connector added to connector index like described here

jinnig and others added 6 commits August 20, 2021 02:26
@jinnig
Added the DynamoDB destination connector.
893338b 
Implemented getConsumer and check methods.

Signed-off-by: Jinni Gu <jinnigu@uw.edu>
@YQ-Wang
Added auto-generated project files.
1974e66 
Signed-off-by: Yiqing Wang <yiqing@wangemail.com>
@YQ-Wang
Added config related files and output table helper.
b7a19d1 
Signed-off-by: Yiqing Wang <yiqing@wangemail.com>
@jinnig
Added document for DynamoDB destination.
2cf22e8 
Signed-off-by: Jinni Gu <jinnigu@uw.edu>
@qtz123
Implemented DynamodbWriter.
dd8d582 
Added integration tests and unit tests.

Signed-off-by: qtz123 <qiutingzhi1995@gmail.com>
@qtz123
Added DynamoDB in the SUMMARY.md.
d8b4cef 
Signed-off-by: qtz123 <qiutingzhi1995@gmail.com>
@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Aug 21, 2021
jinnig and others added 3 commits August 21, 2021 00:05
@jinnig
Merge branch 'master' into dynamodb-destination
125203f
@jinnig
Formatted code using ./gradlew format.
14954d5 
Signed-off-by: Jinni Gu <jinnigu@uw.edu>
@qtz123
Added changelog to the doc.
843dcee 
Signed-off-by: qtz123 <qiutingzhi1995@gmail.com>
@marcosmarxm
Copy link
Member

thanks for the contribution @jinnig

@marcosmarxm marcosmarxm changed the title New Destination: DynamoDB 🎉 New Destination: DynamoDB Aug 24, 2021
@marcosmarxm marcosmarxm self-requested a review August 24, 2021 22:54
@marcosmarxm marcosmarxm self-assigned this Aug 24, 2021
marcosmarxm
marcosmarxm requested changes Aug 25, 2021
View reviewed changes
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Already test locally and use with the webapp and works great! Some small comments let me know what do you think about them @jinnig

...tion-dynamodb/src/main/java/io/airbyte/integrations/destination/dynamodb/DynamodbWriter.java Outdated Show resolved Hide resolved
...tion-dynamodb/src/main/java/io/airbyte/integrations/destination/dynamodb/DynamodbWriter.java Outdated Show resolved Hide resolved
...db/src/main/java/io/airbyte/integrations/destination/dynamodb/DynamodbOutputTableHelper.java Show resolved Hide resolved
@YQ-Wang
Used PAY_PER_REQUEST instead of provisioned for DynamoDB.
23fa643 
Gave the value a name batchSize.
Removed unnecessary logs.

Signed-off-by: Yiqing Wang <yiqing@wangemail.com>
marcosmarxm
marcosmarxm reviewed Aug 25, 2021
View reviewed changes
...ion-dynamodb/src/main/java/io/airbyte/integrations/destination/dynamodb/DynamodbChecker.java
try {
PutItemOutcome outcome = table
.putItem(
new Item().withPrimaryKey(JavaBaseConstants.COLUMN_NAME_AB_ID, UUID.randomUUID().toString(), "sync_time", System.currentTimeMillis()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jinnig Dynamodb is a key-value and document database. Using the primary key as a hash type means you're focusing this connector as a document database? Or a hash type PK using in key-value for Dynamodb is not a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now we are just using DynamoDB as a document DB to store JSON objects. However, we have discussed this topic with Shrif - see how to use getPrimaryKey() to get a primary key. Do you think the other issues have been resolved in the last commit?

@jinnig jinnig requested a review from marcosmarxm August 26, 2021 07:26
marcosmarxm
marcosmarxm approved these changes Aug 27, 2021
View reviewed changes
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT @jinnig awesome work!

@marcosmarxm marcosmarxm merged commit 273fe86 into airbytehq:master Sep 2, 2021
@jrhizor jrhizor mentioned this pull request Sep 3, 2021
Merged
@marcosmarxm marcosmarxm mentioned this pull request Sep 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcosmarxm marcosmarxm marcosmarxm approved these changes

Assignees

@marcosmarxm marcosmarxm

Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/destination/dynamodb connectors/destinations-database
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jinnig @marcosmarxm @karinakuz @YQ-Wang @qtz123