-
Notifications
You must be signed in to change notification settings - Fork 183
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: Silence home directory warning in Lambda Extensions #1344
fix: Silence home directory warning in Lambda Extensions #1344
Conversation
This PR follows on to smithy-lang#893 which added LAMBDA_TASK_ROOT check. When running as a [Lambda Extension][ext] the environment variable for TASK_ROOT is redacted, so it is not reliable for testing whether you are in Lambda. Related commit: fbb4bc [ext]: https://docs.aws.amazon.com/lambda/latest/dg/runtimes-extensions-api.html
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.
Good find. Thank you for the contribution! I want to examine the set of environment variables we have available to us a little more to make sure we choose the right ones, but otherwise, it looks good to go.
environment.get("LAMBDA_TASK_ROOT").is_ok() | ||
// AWS_LAMBDA_RUNTIME_API - Double-check because LAMBDA_TASK_ROOT is redacted to any code running in Extensions | ||
// https://docs.aws.amazon.com/lambda/latest/dg/runtimes-extensions-api.html | ||
environment.get("LAMBDA_TASK_ROOT").is_ok() || environment.get("AWS_LAMBDA_RUNTIME_API").is_ok() |
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.
From what I can tell in the docs, AWS_LAMBDA_RUNTIME_API
is only available when using a custom runtime. Perhaps we should check AWS_LAMBDA_FUNCTION_NAME
instead of either of these values. Do you know if AWS_LAMBDA_FUNCTION_NAME
is set for extensions also?
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.
AWS_LAMBDA_RUNTIME_API
is always set in a Lambda extension process, it is required for the communication between the extension and the runtime.
I'll check on AWS_LAMBDA_FUNCTION_NAME
and get back to you, if that works for both then we can reduce this back down to one check.
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.
Yep, AWS_LAMBDA_FUNCTION_NAME
exists for both, so I've switched the check to use 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.
Looks great! Thank you!
Motivation and Context
Similar to aws-sdk-rust #307, suppress the home directory expansion warning when in Lambda. The fix #893 worked for Lambda functions, but not for Extensions.
Related commit: fbb4bc
Description
This PR follows on to #893 which added
LAMBDA_TASK_ROOT
check. When running as a Lambda Extension the environment variable forLAMBDA_TASK_ROOT
is redacted, so it is not reliable for testing whether you are in Lambda.To remedy this, I switched the check to use
AWS_LAMBDA_FUNCTION_NAME
so that the check returns correctly for both function handlers and extensions.Testing
Ran the full test suite, and validated that this suppresses the warning by rebuilding my local SDK and using it in a Lambda Extension.
Checklist
CHANGELOG.next.toml
if I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.toml
if I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.