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

Add support for specifying a custom Logical ID for the AWS::AppSync::GraphQLApi resource #618

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

manwaring
Copy link

@manwaring manwaring commented Oct 3, 2023

Exploring a possible solution to #617 by supporting a parameter to prefix the AWS::AppSync::GraphQLApi Logical ID (instead of only supporting GraphQlApi)

  • Update plugin schema/types
  • Add optional override property
  • Add unit tests
  • Confirm if approach is acceptable to library creator/maintainer

@@ -104,6 +106,6 @@ export class Naming {
}

getAuthenticationEmbeddedLamdbaName() {
return `${this.apiName}Authorizer`;
return `${this.config.name}Authorizer`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary?

Unless I am missing something, we probably could get away with having a
apiLogicalId that overrides getLogicalId.

The rest of the logical ids should be safe. i.e. They might be re-created but they are not stateful.

That would simplify everything.

@@ -12,6 +12,7 @@ export type AppSyncConfig = {
pipelineFunctions: Record<string, PipelineFunctionConfig>;
substitutions?: Substitutions;
xrayEnabled?: boolean;
logicalIdPrefix?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As per my other comment, could this be named apiLogicalId?

Copy link
Collaborator

@bboure bboure left a comment

Choose a reason for hiding this comment

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

Thank you @manwaring

Could we also add documentation of when and how to use it?

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

2 participants