-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore(secretsmanager): Remove unused secretName attribute #10410
Conversation
* | ||
* @default - the name is derived from the secretArn. | ||
*/ | ||
readonly secretName?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would argue that a better DX is to only support secretName
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to only support secretName
We can't stop supporting secretArn
as an attribute, no.
You mean make secretArn
optional, and add back in secretName
as optional, then a validation in fromSecretAttributes
that at least one is present? We can do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to only support secretName
We can't stop supporting
secretArn
as an attribute, no.You mean make
secretArn
optional, and add back insecretName
as optional, then a validation infromSecretAttributes
that at least one is present? We can do that.
I guess that could work although generally against our api guidelines (normally in such cases we prefer something like union-like classes). I am okay with how it is now
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
In #10309, secretName was added to SecretAttributes, but given the ARN is always
required, it's fairly redundant. Removing to reduce public API surface area.
Not a breaking change, as #10309 has not yet been released.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license