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

fix: Change 'rds' to 'sql' in public-facing symbols #2069

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

palpatim
Copy link
Member

Description of changes

This change leaves most of the internal symbols unchanged, but should pick up the public-facing ones. Notably:

  • Imported schema file name
  • Stack name
  • SQL related CDK resource IDs
  • Some error & warning messages

This PR does NOT override ‘RDS’ in resolver template output, since that would bloat the size of the PR and those files aren't generally reviewed by customers anyway.

Checklist

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

@palpatim palpatim requested review from a team as code owners November 14, 2023 23:11
RDSPatchingLambdaLogAccessPolicy: 'RDSPatchingLambdaLogAccessPolicy',
RDSPatchingLambdaLogicalID: 'RDSPatchingLambdaLogicalID',
RDSLambdaDataSourceLogicalID: 'RDSLambdaDataSource',
RDSLambdaDataSourceLogicalName: 'RDSLambdaDatabase',
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I removed this constant -- it was not in use anywhere and was easily confused with RDSLambdaDataSourceLogicalID.

SQLPatchingLambdaLogAccessPolicy: 'SQLPatchingLambdaLogAccessPolicy',
SQLPatchingLambdaLogicalID: 'SQLPatchingLambda',
SQLLambdaDataSourceLogicalID: 'SQLLambdaDataSource',
SQLPatchingSubscriptionLogicalID: 'SQLPatchingSubscription',
Copy link
Member Author

@palpatim palpatim Nov 14, 2023

Choose a reason for hiding this comment

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

Note that I added SQLPatchingSubscriptionLogicalID and SQLPatchingTopicLogicalID, which were previously hardcoded in the subscription creation logic

Copy link
Member Author

@palpatim palpatim left a comment

Choose a reason for hiding this comment

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

marker review

alharris-at
alharris-at previously approved these changes Nov 14, 2023
Comment on lines +61 to +62
// For beta use account '956468067974', layer name 'AmplifySQLLayerBeta' and layer version '12' as of 2023-06-20
// For prod use account '582037449441', layer name 'AmplifySQLLayer' and layer version '3' as of 2023-06-20
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker, but comment is out of date.

@palpatim palpatim enabled auto-merge (squash) November 15, 2023 00:35
@palpatim palpatim merged commit ff374dd into main Nov 15, 2023
5 of 6 checks passed
@palpatim palpatim deleted the palpatim.fix.change-rds-to-sql branch November 15, 2023 00:35
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.

3 participants