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

feat: allow layer deployment for nodejs #4112

Conversation

hirochachacha
Copy link
Contributor

This CL allows users to deploy lambda layer manually.
The interactive shell is missing, only nodejs support, but it should be
enough for the initial support.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This CL allows users to deploy lambda layer manually.
The interactive shell is missing, only nodejs support, but it should be
enough for the initial support.
@codecov-io
Copy link

Codecov Report

Merging #4112 into master will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4112      +/-   ##
==========================================
+ Coverage   60.64%   60.75%   +0.11%     
==========================================
  Files         327      326       -1     
  Lines       14359    14332      -27     
  Branches     2858     2850       -8     
==========================================
  Hits         8708     8708              
+ Misses       5215     5188      -27     
  Partials      436      436              
Impacted Files Coverage Δ
packages/amplify-util-mock/src/mockAll.ts

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf476e2...4aa553f. Read the comment docs.

@jhockett
Copy link
Contributor

jhockett commented May 1, 2020

Hi @hirochachacha, we're looking to create a more comprehensive experience with all supported runtimes and a few other features on Lambda Layer release. This would unfortunately conflict with our Lambda Layers implementation and could cause issues between CLI versions if we merged this beforehand.

I totally understand that you (and many others!) want this feature asap, but we are still diligently working on building out this feature. As a temporary work-around you could use your personal fork and run a locally built amplify-dev.

@hirochachacha
Copy link
Contributor Author

@jhockett

Hi @hirochachacha, we're looking to create a more comprehensive experience with all supported runtimes and a few other features on Lambda Layer release. This would unfortunately conflict with our Lambda Layers implementation and could cause issues between CLI versions if we merged this beforehand.

Are you worried about concrete issue or hypothetical one?
Actually I can easily create comprehensive design by myself, but I don't think you guys will accept the PR. I've been sending few PRs to this project since 4 months ago, and I learned something. You guys are unwilling to accept PRs outside of the team. right? Trivial one is OK, but influential one is always ignored. I feel like I always hit NIH.
That's why I tried to start from small one here, but it looks like I failed it again. No idea.
Could you tell how can I convince you? Or is it impossible as I predicted?

I totally understand that you (and many others!) want this feature asap, but we are still diligently working on building out this feature. As a temporary work-around you could use your personal fork and run a locally built amplify-dev.

Of course, we have been using the temporary work-around for 4 months. What's the definition of comprehensive experience? We don't see any problems so far.

@hirochachacha
Copy link
Contributor Author

@edwardfoyle Could you show me your plan? Will it conflict with this? Whether there're other ideas, I thought deployment part is almost same.

@hirochachacha
Copy link
Contributor Author

Supporting python should be trivial. dotnet needs aws/aws-extensions-for-dotnet-cli#79. java uses gradle, so replacing 'lib' with 'java/lib' is good enough. https://github.com/aws-amplify/amplify-cli/blob/master/packages/amplify-java-function-template-provider/resources/lambda/hello-world/build.gradle.ejs#L24. go doesn't have stable implementation of shared library so far. Deployment feature and interactive shell feature are orthogonal. Where did you find the confliction?

@hirochachacha
Copy link
Contributor Author

And ruby? hirochachacha@c14ba43

@renebrandel
Copy link
Contributor

@hirochachacha - first off, I want to thank you for contributing to Amplify CLI and pushing the boundaries of what's possible. We are actively working on Lambda Layer support and hopefully we can share more about it soon with you.

One element that we find really valuable in all this is the Ruby support for Lambda functions that you've developed. If you're interested to upstream that into the Amplify CLI codebase, you can create a separate PR just for Ruby support. @jhockett and I can work with you to get that functionality integrated into Amplify CLI.

We love your contributions. We recommend you to create an issue before contributing large PRs though as there are many moving pieces that we need to align. Please refer to the contribution guidelines for more details: https://github.com/aws-amplify/amplify-cli/blob/master/CONTRIBUTING.md

@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants