-
Notifications
You must be signed in to change notification settings - Fork 4k
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(lambda): function.addAlias()
simplifies Alias creation
#20034
Conversation
Deprecate `version.addAlias()` in favor of a new method called `function.addAlias()`. - When Aliases get created underneath Versions, there's the risk that it's `currentVersion` whose logical ID changes on every deployment, causing a (probably failing) replacement of the `Alias` on every function update. - The most common use for `function.currentVersion` is to create an `Alias` for a Function. Might as well take out the middle man.
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.
When Aliases get created underneath Versions, there's the risk
that it's currentVersion whose logical ID changes on every
deployment, causing a (probably failing) replacement of the Alias
on every function update.
The most common use for function.currentVersion is to create an
Alias for a Function. Might as well take out the middle man.
I'm a bit confused here -- see my inline comments below. The above reads
like the common use case for fn.currentVersion.addAlias()
will cause
potentially failing replacement of the Alias on every function update. If so,
I'm unclear how the code changes solves this -- I trust you, I just need it
to be pointed out :).
@@ -547,6 +547,18 @@ describe('function', () => { | |||
Annotations.fromStack(stack).hasNoWarning('/Default/MyLambda/$LATEST', Match.stringLikeRegexp(warningMessage)); | |||
}); | |||
|
|||
test('function.addAlias', () => { |
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.
If I'm understanding correctly, the issue with fn.currentVersion.addAlias()
is that the Alias will be replaced on every function update. I think we should test that fn.addAlias()
solves the issue. This test doesn't really tell me anything.
To be honest, I'm not clear how function.addAlias()
changes this behavior. To me, it seems like both fn.addAlias()
and fn.currentVersion.addAlias()
results in the same thing: return addAlias(this, <current-version>, aliasName, options)
.
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.
Yes, but the this
is different! In one case it's the Function
and the other it's the Version
.
But I think I was mistaken anyway: I thought the Alias
would be replaced if the Version
object changed... but I'm now pretty convinced it doesn't.
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.
So this is just convenience.
Co-authored-by: Kaizen Conroy <36202692+kaizen3031593@users.noreply.github.com>
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). |
) Deprecate `version.addAlias()` in favor of a new method called `function.addAlias()`. - When Aliases get created underneath Versions, there's the risk that it's `currentVersion` whose logical ID changes on every deployment, causing a (probably failing) replacement of the `Alias` on every function update. - The most common use for `function.currentVersion` is to create an `Alias` for a Function. Might as well take out the middle man. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Deprecate
version.addAlias()
in favor of a new method calledfunction.addAlias()
.that it's
currentVersion
whose logical ID changes on everydeployment, causing a (probably failing) replacement of the
Alias
on every function update.
function.currentVersion
is to create anAlias
for a Function. Might as well take out the middle man.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license