search

aws / aws-lambda-runtime-interface-emulator

Apache License 2.0
921 stars 95 forks source link

Fix default function name #49

Closed skwashd closed 2 years ago

skwashd commented 3 years ago

Issue #, if available:

Description of changes:

As noted in #46, the function name is derived from the URL used to invoke it. The emulator uses function in the API endpoint, but the AWS_LAMBDA_FUNCTION_NAME environment variable is set to test_function. This is an inconsistency between the emulator and the AWS environment.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

valerena commented 2 years ago

@skwashd thanks for this PR. Even though this change makes sense, it's actually changing the current "default" behavior, so I'll have to confirm with the team to see if we need to handle this in a special way. We'll get back to you after that.

skwashd commented 2 years ago

@valerena thanks for the feedback. While rerolling #46, I realised those changes depend on this fix. I'm happy for us to close this and deal with both issues together over on on the other PR if that's easier.

If we were to follow semantic versioning, this fix would be a backwards compatibility break and so require a major release. I get that's not fun. The current behaviour is a bug as it doesn't follow the documented behaviour of the AWS Lambda environment.

I double checked the API docs and the Lambda environment variable docs and they align with this fix. The function name in the API call path should match the value of the AWS_LAMBDA_FUNCTION_NAME environment variable.

valerena commented 2 years ago

@skwashd I agree that it's better to handle both together. The other change is still "breaking", considering that for example SAM cli currently handles the fact that RIE always uses /function/invocations, so it's something that will have to be handled on that side before consuming the new version of RIE.

I think we could merge the changes into develop relatively soon, but we'll wait before actually making the new release.

Thanks for the contribution.

skwashd commented 2 years ago

I've cherry-picked this commit onto the #46 PR. I'll let you decide if we close this one or merge this then the other one.

skwashd commented 2 years ago

@valerena I was wondering if you'd made any progress with the team on this one

valerena commented 2 years ago

Sorry, I thought I added a comment here. I'll close this one and let's keep everything in the other PR.

We haven't been able to fully get to this, but we should be able to close all our discussions soon. Feel free to ping us again in the other PR if by the end of next week there hasn't been any movement.