Skip to content
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

enhancements for AWS lambda #14

Closed
wants to merge 9 commits into from
Closed

enhancements for AWS lambda #14

wants to merge 9 commits into from

Conversation

leoholz
Copy link
Collaborator

@leoholz leoholz commented Feb 28, 2020

This branch debugs the AWS support and adds configuration of environment vars and layers.

@NathanTP
Copy link
Contributor

NathanTP commented Mar 2, 2020

Can you include documentation in docs/ for the user-visible aspects of these changes? At a minimum, it would help me in understanding what changes are being made here and the objectives of those changes. In particular: what are layers?

@NathanTP
Copy link
Contributor

NathanTP commented Mar 2, 2020

Also, you may want to merge master, I made some changes to the docs in #13

@leoholz
Copy link
Collaborator Author

leoholz commented Mar 2, 2020

Can you include documentation in docs/ for the user-visible aspects of these changes? At a minimum, it would help me in understanding what changes are being made here and the objectives of those changes. In particular: what are layers?

Of course, thanks for the hint. Please have a look at https://docs.aws.amazon.com/lambda/latest/dg/configuration-layers.html regarding layers.

I have to admit that I am not sure if I integrated the layers in the right way, but they are basically part of the function provider. If you have other ideas how to integrate them let me know.

@leoholz
Copy link
Collaborator Author

leoholz commented Mar 3, 2020

master is merged and I added a bit of documentation at Examples.

Copy link
Collaborator

@jssmith jssmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes seem mostly reasonable. The one high-level question that I have is whether we are approaching the addition of layers in the right way. At present it is 1:1 mapping with how they work in Lambda. I wonder whether this is right, or whether having multiple base images for the FaaS provider might be a better design. @NathanTP, I'd love to hear your thoughts on this. In the interest of keeping things moving I'm inclined to accept the current approach but maybe we should stay open to readdressing it.

@@ -32,6 +32,6 @@ service :
# e.g. `arn:aws:iam::123459789012:role/service-role/my-service-role-ae04d032`
role : null
# Optional vpc/security-group setup to use.
# e.g.: "vpc-123456789abcdef,sg-123456789abcdef"
# e.g.: "subnet-8910f2f5,sg-123456789abcdef"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not change this example.

@leoholz
Copy link
Collaborator Author

leoholz commented Mar 17, 2020

closing PR as current version is #15

@leoholz leoholz closed this Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants