Skip to content

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

Merged
merged 5 commits into from
Dec 1, 2021

Conversation

mLupine
Copy link
Contributor

@mLupine mLupine commented Nov 20, 2021

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.

@pgrzesik
Copy link
Contributor

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! 🙇

Maciej Wilczyński added 2 commits November 30, 2021 15:02
* 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
  ...
@mLupine
Copy link
Contributor Author

mLupine commented Nov 30, 2021

Hi @pgrzesik, branch updated.

Copy link
Contributor

@pgrzesik pgrzesik left a 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

@mLupine
Copy link
Contributor Author

mLupine commented Nov 30, 2021

That's weird, when I run eslint . or npm run lint locally, it doesn't find any formatting errors. I'll dig into it tomorrow.

@pgrzesik
Copy link
Contributor

@mLupine From what I see it's prettier that is complaining, you might want to run npm run prettify

@mLupine
Copy link
Contributor Author

mLupine commented Dec 1, 2021

@pgrzesik Indeed, missed that. Should be fine now :)

@mLupine mLupine requested a review from pgrzesik December 1, 2021 11:06
@mLupine
Copy link
Contributor Author

mLupine commented Dec 1, 2021

@pgrzesik Well, the check failed again. Seems that npm run prettify for some reason skips files within the lib/ directory. I ran prettier manually, now it should really be fine 😉

Copy link
Contributor

@pgrzesik pgrzesik left a 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!

@pgrzesik pgrzesik changed the title Add architecture to requirements cache directory name feat: Add architecture to requirements cache directory name Dec 1, 2021
@pgrzesik pgrzesik merged commit 328cb01 into serverless:master Dec 1, 2021
mLupine pushed a commit to mLupine/serverless-python-requirements that referenced this pull request Dec 2, 2021
* master:
  feat: Add architecture to requirements cache directory name (serverless#645)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants