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.

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

feat: add RDS primary key transformer #1216

Merged
merged 5 commits into from
Feb 1, 2023

Conversation

phani-srikar
Copy link
Contributor

@phani-srikar phani-srikar commented Jan 31, 2023

Description of changes

  • Update the primary key index transformer to add support for generating appropriate AppSync pipeline resolvers for RDS imported data source.
  • Update the schema generation to handle partition and sort key fields in the primary key.
CDK / CloudFormation Parameters Changed

Issue #, if available

Description of how you validated changes

Expanded the current unit tests to be run for RDS primary key transformer.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)
  • New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies
  • Any CDK or CloudFormation parameter changes are called out explicitly

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@phani-srikar phani-srikar requested a review from a team as a code owner January 31, 2023 06:46
@phani-srikar phani-srikar changed the title feat: add RDS primary key index transformer feat: add RDS primary key transformer Jan 31, 2023
@phani-srikar phani-srikar force-pushed the rds-primary-key-index-transformer branch from 0bedac2 to 26360c3 Compare January 31, 2023 19:58
@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2023

Codecov Report

Merging #1216 (97e6ca9) into feature/rds-support (97e6ca9) will not change coverage.
The diff coverage is n/a.

❗ Current head 97e6ca9 differs from pull request most recent head 17a9e06. Consider uploading reports for the commit 17a9e06 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@                 Coverage Diff                  @@
##           feature/rds-support    #1216   +/-   ##
====================================================
  Coverage                63.42%   63.42%           
====================================================
  Files                      297      297           
  Lines                    18931    18931           
  Branches                  4573     4573           
====================================================
  Hits                     12007    12007           
  Misses                    6309     6309           
  Partials                   615      615           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@marcvberg marcvberg left a comment

Choose a reason for hiding this comment

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

Minor nits, overall LGTM

Comment on lines +39 to +61
if (getResolver) {
addIndexToResolverSlot(getResolver, [primaryKeySnippet]);
}

if (listResolver) {
const sortDirectionValidation = printBlock('Validate the sort direction input')(compoundExpression(validateSortDirectionInput(config, true)));
addIndexToResolverSlot(listResolver, [
primaryKeySnippet,
sortDirectionValidation
]);
}

if (createResolver) {
addIndexToResolverSlot(createResolver, [primaryKeySnippet]);
}

if (updateResolver) {
addIndexToResolverSlot(updateResolver, [primaryKeySnippet]);
}

if (deleteResolver) {
addIndexToResolverSlot(deleteResolver, [primaryKeySnippet]);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these for RDS?

Copy link
Contributor Author

@phani-srikar phani-srikar Feb 1, 2023

Choose a reason for hiding this comment

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

Yes we do, in order to set the keys metadata which is later used in lambda to set WHERE conditions based on the columns used in primary key.

@phani-srikar phani-srikar force-pushed the rds-primary-key-index-transformer branch from c7667cf to 17a9e06 Compare February 1, 2023 22:41
@phani-srikar phani-srikar merged commit a08589a into feature/rds-support Feb 1, 2023
@phani-srikar phani-srikar deleted the rds-primary-key-index-transformer branch February 1, 2023 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants