-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add lambda config to ignore architecture #7890
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
Conversation
37684bc
to
965d2ae
Compare
965d2ae
to
a8bdc0e
Compare
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.
LGTM!
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.
LGTM, just some minor questions for tests, nothing to hold back the merge though.
@pytest.mark.skipif( | ||
not use_docker(), | ||
reason="Monkeypatching of Docker-related flag not applicable if run locally", | ||
) |
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.
Why wouldn't this work?
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.
Monkeypatching works but Docker-flags are not used with the local executor in the old lambda provider because no new Docker container is started. Hence, DOCKER flags are not applicable and Docker tests with the local executor fail.
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.
Ah, now i understand. The if run locally
confused me, as I thought of running the test locally, not running lambdas in the local executor. Overlooked the use_docker
and thought of in_docker
.
Still, use_docker might be problematic, as it would skip the tests if I set (by accident) something weird like LAMBDA_EXECUTOR=local PROVIDER_OVERRIDE_LAMBDA=v2
, which should work (as it uses the new provider) but will skip as it thinks of the old provider only.
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.
I clarified the reason and added a new is_old_local_executor()
util. This skip is currently redundant for test_ignore_architecture
given the old provider is unsupported anyways but needs to be extended to the new local executor replacement (i.e., the self-managed workers) because they do not spawn new containers either.
@pytest.mark.skipif( | ||
not is_arm_compatible() and not is_aws(), | ||
reason="ARM architecture not supported on this host", | ||
) |
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.
I think this is fine for now, but emulation is thinkable in both ways, also arm64 on amd64.
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.
interesting point. That could even be added to is_arm_compatible()
if it becomes relevant in the future.
Use standardized LocalStack `get_arch` to match Linux platform.
a8bdc0e
to
9df0b23
Compare
Add an environment variable
LAMBDA_IGNORE_ARCHITECTURE
to ignore the architecture (i.e., x86_64 or arm64) of a Lambda function and let Docker select a matching image based on the host operating system and architecture.Motivation
amd64_only
andarm64_only
).Further changes
get_arch()
to matchaarch64
platform on Linuxis_arm_compatible()
to platform utilsPULLED_IMAGES
cache platform-aware to prevent running into timeouts upon invocation