Zappa
Zappa copied to clipboard
Add Support for Graviton 2 / ARM `Architectures`
Description
This PR adds support for Graviton 2 / ARM64 Lambda's. It adds a new architectures
configuration option, which defaults to x86_64
. This parameter is also used for correct architecture wheel selection.
This is my first submission to Zappa, so please do CR and feedback anything I might have missed, or need to do.
AWS API Docs
GitHub Issues
Resolves: https://github.com/zappa/Zappa/issues/1051
@hellno @javulticat
How do we get PR's reviewed? Is there a group / person we should tag?
From experience allowing PR's to go stale discourages contributions
Looks great overall, a small naming convention (I don't want to spam it on every line 😅):
Why is the parameter name
architectures
?In the settings I can only select one and it is being stored as a list, but when used, you take only ever take the first object (
architectures[0]
).I might be missing something here, sorry for my ignorance if that's the case.
Happy to discuss and get this merged, sorry for the CR delay
Thanks for the feedback, and no worries on the delay, I just wasn't sure if I needed to tag someone / CR Group.
The only reason I used 'architectures' is to be consistent with the AWS API Terminology. I do see your point and will happily change it, I guess the same is true for having it as a List, again that was mirroring the API, but as long as I pass it in as a list, we're good.
If you agree I can go ahead and make those updates, in all honesty I wasn't too clear on what conventions I should be using, but that's all part of contributing 👍
To Summarise:
- Remove pluralisation of architecture parameter
- Change type from List to String
Yes, that sounds great! thanks for the effort 💪🏼
I'll work on this at the weekend, been busy!
@turingbeing , do you need help on this PR ?
I want to use this feature. How can I help this to get merged?
I want to use this feature. How can I help this to get merged?
You should be able to use it from the branch without merging it, just needs some minor refactoring, but been mega busy with life and work to do it
Actually I think it's here: https://github.com/zappa/Zappa/pull/1123
@turingbeing Looks like this would be nice to get merged in.
Do you have time to address the comments in the coming months and rebase off of master?
This PR is further along, closing this one.
https://github.com/zappa/Zappa/pull/1123