-
Notifications
You must be signed in to change notification settings - Fork 144
feat: Rust cargo lambda workflow #350
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
dd7b531
to
758275a
Compare
@jfuss can you help me figure out what's going on with Appveyor? I think at this point none of the errors I'm seeing are due to the changes in this PR. |
On the Linux Appveyor jobs, it seems all tests passed but the format check failed due to I'm still investigating the Windows Appveyor jobs. Will update later.
|
acc8798
to
f28022c
Compare
ae288f1
to
6840d87
Compare
ca60d67
to
a7a4138
Compare
Windows checks have been failing with following error:
|
Yup, I'm aware of the error, I just don't know how to reproduce it unfortunately because the tests pass in my windows laptop. I have more information here if you're curious: cargo-lambda/cargo-lambda#77 |
Can you check if you disable (or extend) this maximum path option for windows? |
Converting the PR to draft as it seems it's still work in progress |
@calavera do you have any updates regarding to this windows failures? I wonder if we can push this change under a feature flag, and noting that Windows might have issues that we are still investigating. Rust is getting traction from the community, and providing a native builder for SAM CLI would be good for our customers. |
@mndeveci I have not been able to reproduce those errors in any other environment unfortunately. I'm all for pushing this. I can make the changes if you tell me how to add a feature flag. |
@mndeveci for what is worth, I have integration tests on Windows that run on GitHub Actions in cargo-lambda. Builds work there without this issue: https://github.com/cargo-lambda/cargo-lambda/runs/7226829676 |
I also saw that Appveyor has a newer windows image, can you give it a try with that? You need to update |
I saw windows ones succeeded in last try but they also took like couple of seconds. There is also a condition below in same file, you need to update there as well;
|
I think that was a missconfiguration, the build took 6 seconds only 🙈 |
I was finally able to reproduce this on my personal laptop. So at least I can investigate why it's happening. |
That is great news! I will be preparing feature flag in SAM CLI so that we can use it with this builder. |
2e0d1f2
to
cea242e
Compare
Signed-off-by: David Calavera <[email protected]>
* feat: Pin ruff version, add dependabot config to keep our dependencies up to date * Exlude isort and flake8 from dependabot updates
Signed-off-by: David Calavera <[email protected]>
Signed-off-by: David Calavera <[email protected]>
Signed-off-by: David Calavera <[email protected]>
Signed-off-by: David Calavera <[email protected]>
Signed-off-by: David Calavera <[email protected]>
Signed-off-by: David Calavera <[email protected]>
6bf691d
to
2237d76
Compare
binary_path = self.binary_path() | ||
destination_path = os.path.join(self._artifacts_dir, "bootstrap") | ||
LOG.debug("copying function binary from %s to %s", binary_path, destination_path) | ||
self._osutils.copyfile(binary_path, destination_path) |
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 did some integration tests and found that this doesn't copy the permission bits of the executable. We should use copy2
instead.
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 for catching that @hawflau ! I've updated the code to use copy2
.
Use copy2 instead of copyfile Signed-off-by: David Calavera <[email protected]>
Awesome work @calavera (and to the reviewers!) 🎉 |
This is a continuation of #174. It updates the Rust workflow to use cargo-lambda, a tool that we've been building to simplify how to compile Rust functions for AWS Lambda. It supports cross-compiling natively, so you don't have to worry about it. The workflow is much more simple now, since it just calls cargo-lambda and copies the resulting artifacts.
I've also updated tests and example projects to work with this new version.
#174 can be closed, since this PR picks up where it left.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.