-
Notifications
You must be signed in to change notification settings - Fork 105
Make invocation URL dynamic and fix default function name #46
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
base: develop
Are you sure you want to change the base?
Changes from all commits
b57d8f4
49c35f6
43a7a09
5a79e1e
9de974f
23f8171
c6779f7
25a2eac
c99378b
394ab66
e3c135c
ff9e4f1
cb1fbb7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,13 +25,13 @@ def check_env_var_handler(event, context): | |
|
||
def assert_env_var_is_overwritten(event, context): | ||
print(os.environ.get("AWS_LAMBDA_FUNCTION_NAME")) | ||
if os.environ.get("AWS_LAMBDA_FUNCTION_NAME") == "test_function": | ||
if os.environ.get("AWS_LAMBDA_FUNCTION_NAME") == "function": | ||
raise("Function name was not overwritten") | ||
else: | ||
return "My lambda ran succesfully" | ||
|
||
def assert_lambda_arn_in_context(event, context): | ||
if context.invoked_function_arn == f"arn:aws:lambda:us-east-1:012345678912:function:{os.environ.get('AWS_LAMBDA_FUNCTION_NAME', 'test_function')}": | ||
if context.invoked_function_arn == f"arn:aws:lambda:us-east-1:012345678912:function:{os.environ.get('AWS_LAMBDA_FUNCTION_NAME', 'function')}": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I found an issue with our tests thanks to this change. Your new test works correctly with this code, but there was already a previous test Looking at the output of the test run I'll have to figure out this issue, but in the meantime you should change this to have different cases depending if the new env var was passed or not (probably just create a new handler here to be called form the new integ test). The previous handler in this file also has the same issue, but because it's the branch that shouldn't happen, then it's not relevant and the test still succeed, but you should change that too. Everything else looks good There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Digging a little deeper, it looks like this shouldn't really be a problem, because when running this we will always have a value for It's still weird that the previous test that uses this code doesn't show up on the output for the tests, so I'm still investigating what's going on there. Did you make this change because the tests failed if you didn't? Or did you change it before actually running the tests? (My point is that: either this change is making the old test fail, or this change shouldn't be needed). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course it had to be a simple thing. The old test is not running because the new test you added has the same name! ( What I would say is: don't change this file. This change doesn't actually affect the test because the env var is always set. We could probably just remove that second parameter in the test, but for now, it's better if you don't modify the file, so it doesn't get mixed up as a needed change. |
||
return "My lambda ran succesfully" | ||
else: | ||
raise("Function Arn was not there") | ||
|
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.
This one should be
test_lambda_function_arn_exists_consistent