Skip to content
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

Remove native client #12

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

calavera
Copy link

@calavera calavera commented Jan 8, 2021

This allows using the runtime interface on Windows or MacOS since it doesn't depend on the Linux native client anymore.

Signed-off-by: David Calavera david.calavera@gmail.com

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

This allows using the runtime interface on Windows or MacOS since it doesn't depend on the Linux native client anymore.

Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: David Calavera <david.calavera@gmail.com>
@calavera
Copy link
Author

calavera commented Jan 8, 2021

Can any of the maintainers help me debug the integration tests? I see an error in the log, but I have no idea how to reproduce it in isolation:

https://github.com/aws/aws-lambda-python-runtime-interface-client/pull/12/checks?check_run_id=1671731745#step:4:1218

[EDIT] I think I managed to solve the problem 🎉

Signed-off-by: David Calavera <david.calavera@gmail.com>
The native client raised exceptions if the response code in not OK.

Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: David Calavera <david.calavera@gmail.com>
@carlzogh
Copy link
Contributor

Hey @calavera - thanks for your contribution!

The native client provides a significant performance boost for the function runtime, and thus we're reluctant to simply remove it.

One thing we would consider is an alternate strategy to use Python's built-in modules if the user explicitly wants it to be so. Potentially this could be implemented either as a fallback for when the native installation fails, or as a separate pip package from the same codebase.

I'm reluctant to automatically default to the built-in Python HTTP client version if the native installation fails, as that could be unintentional where users might have a couple of missing dependencies and don't realize that they ended up with the less performant installed version of the two variants. I'd rather this be a conscious decision for the user to make instead.

@calavera
Copy link
Author

The native client provides a significant performance boost for the function runtime, and thus we're reluctant to simply remove it.

that's interesting. Do you have metrics about it? I understand your concerns, I'm curious because I've only seen this interface, and the javascript one using that same native client, while other runtimes, like ruby, rust, and go use their language http stack. To be honest, I'm not super familiar with Python.

Potentially this could be implemented either as a fallback for when the native installation fails, or as a separate pip package from the same codebase.

I'm going to take a look at what this could entail.

I'd rather this be a conscious decision for the user to make instead.

got it

@carlzogh
Copy link
Contributor

Do you have metrics about it?

Performance test comparisons showed latency reductions of around 30% at p50 and 25% at p99.

I've only seen this interface, and the javascript one using that same native client, while other runtimes, like ruby, rust, and go use their language http stack.

The NodeJS Runtime Interface Client follows the same approach, yes - and a similar resolution for an alternative language-native client installation (without native cURL bindings) could be considered there as well! We've opted not to do this for the Ruby client for the time being, but it's surely something we could consider further down the line for similar performance improvements.

Thanks again for your interest in contributing to this library! :)

@calavera
Copy link
Author

and a similar resolution for an alternative language-native client installation (without native cURL bindings) could be considered there as well!

😉 😄

aws/aws-lambda-nodejs-runtime-interface-client#8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants