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

NH-64716 Fix lambda extension setup and OboeAPI call #224

Merged
merged 4 commits into from
Nov 20, 2023

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Nov 18, 2023

Fixes lambda extension linking by adding lambda env var checks to CustomBuildExt.run() in setup.py. The Make target wrapper stays the same. The symlinks (liboboe.so and liboboe.so.0) are created to point to the appropriate, already-downloaded c-lib .so at time of APM Python installation, including when run on local. This makes it easier to test lambda behaviour on local as long as the two LAMBDA env vars are set in the test environment. This stops SSL issue and incorrect sampling decisions I was seeing during testing.

I also simplified setup.py by removing CustomBuildExtLambda and its uses, and moving what it did into CustomBuildExt. Lambda layer builds work by using CustomBuildExt now.

Fixes OboeAPI call to use the same args as when calling Context to get sampling decision.

Tests pass at 63ab599, then I reverted the c-lib 14 stg commit.

@tammy-baylis-swi tammy-baylis-swi marked this pull request as ready for review November 18, 2023 01:13
@tammy-baylis-swi tammy-baylis-swi requested a review from a team as a code owner November 18, 2023 01:13
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice simplification of setup.py @tammy-baylis-swi! The failed test run seems like a blip?

@tammy-baylis-swi
Copy link
Contributor Author

Thanks @cheempz !

The failed test run seems like a blip?

The tests are failing because they're run with c-lib 13 instead of 14 from staging. Is it worth it to add catches/switches in the tests so that e.g. parts are skipped if c-lib < 14? Or allow fails until 14 is released?

@tammy-baylis-swi tammy-baylis-swi merged commit b118dbc into main Nov 20, 2023
3 of 11 checks passed
@tammy-baylis-swi tammy-baylis-swi deleted the NH-64716-update-oboe-api-make-and-call branch November 20, 2023 22:28
@cheempz
Copy link
Contributor

cheempz commented Nov 20, 2023

Thanks @cheempz !

The failed test run seems like a blip?

The tests are failing because they're run with c-lib 13 instead of 14 from staging. Is it worth it to add catches/switches in the tests so that e.g. parts are skipped if c-lib < 14? Or allow fails until 14 is released?

Ah, I think we're close enough to liboboe 14 release that it's not worth changing the tests.

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

Successfully merging this pull request may close these issues.

2 participants