-
Notifications
You must be signed in to change notification settings - Fork 293
feat: Add architecture to requirements cache directory name #645
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
Hey @mLupine 👋 Thanks a lot for PR proposal - could you rebase it on top of the current master? Also I see that linting CI step is failing, could you look into that? Thanks in advance! 🙇 |
* master: (24 commits) chore: Remove Node16 tests chore: Remove dependabot chore: Reformat with eslint & prettier ci: Add commitlint job to CI ci: Introduce new CI publish workflow ci: Introduce integrate CI workflow ci: Update validate CI workflow refactor: Use `ServerlessError` in `pip` refactor: Use `ServerlessError` in `pipenv` refactor: Use `ServerlessError` in `poetry` refactor: Use `ServerlessError` in `docker` refactor: Cleanup and use `finally` for code simplification refactor: Ensure proper verbose progress logs refactor: Adapt `docker` for modern logs refactor: Adapt `inject` to modern logs refactor: Adapt `layer` to modern logs refactor: Adapt `pip` to modern logs refactor: Adapt `zip` to modern logs refactor: Adapt `shared` to modern logs refactor: Adapt `clean` to modern logs ...
Hi @pgrzesik, branch updated. |
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.
Hey @mLupine - could you also please check formatting? It looks like it's failing :( The PR looks good in general though 👍 Let's clear that out and merge it
That's weird, when I run |
@mLupine From what I see it's |
@pgrzesik Indeed, missed that. Should be fine now :) |
@pgrzesik Well, the check failed again. Seems that |
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.
Thanks @mLupine 🙇 As for prettier
, I'll look into that, thanks!
* master: feat: Add architecture to requirements cache directory name (serverless#645)
Now that Lambdas can be run on arm64 runtimes, Python dependencies with native bindings compiled on x86_64 hosts might not work on Graviton lambdas. This can generate quite a lot of issues with dependencies — for example when using LocalStack on x86_64 but deploying to arm64 runtimes.
This PR adds a suffix with architecture name to the cache directory path so that if dependencies are built for multiple architectures on a single computer, they won't be conflicting each other and there'll be no need to empty cache between those builds.
Side note: If #644 gets merged before this,
getRequirementsLayerPath
will also need to be adjusted.