lifadev / archive_aws-lambda-go-shim

Author your AWS Lambda functions in Go, effectively.
https://github.com/eawsy/aws-lambda-go-shim
Apache License 2.0
789 stars 66 forks source link

README Step 3 Deploy is confusing #23

Closed tleyden closed 7 years ago

tleyden commented 7 years ago

It says:

3 - Deploy Runtime: python2.7 Handler: handler.Handle

what does this mean?

I'm looking at my previous commits to see how I got aws-lambda-go-shim working in the past .. and I see that I added a .yaml file, but I don't even see any .yaml files mentioned in the aws-lambda-go-shim README. Have things changed and .yaml files are no longer needed?

fsenart commented 7 years ago

@tleyden yaml file may be related to something like CloudFormation, etc (like when using aws-lambda-go-net). The above details are just for configuring AWS Lambda (which runtime to choose, which Handler to set, etc.) The problem is that this project in particular does not enforce any specific way for deploying your Lambda, also we provide the bare minimum info related to the raw AWS Lambda console configuration. Any other explanation is welcome, I don't know how to improve this section but remain open to your suggestions.

fsenart commented 7 years ago

In the referenced commit, you use the SAM model with the other project aws-lambda-go-net and the settings are in the yaml file aws_serverless_application_model.yaml line 7-8.

fsenart commented 7 years ago

@tleyden PS: long time ago I've submitted small modifications to your project in order to update info about the shim :smile:

tleyden commented 7 years ago

Ok thanks!

I was ignoring aws-labmda-go-net because I thought it was related to some .net stuff .. which actually makes no sense! 🙈

I'll look that at that repo again. (in my mind I will try to think of that repo as aws-lambda-go-api-gateway, even if that's not 100% correct)

I totally missed your PR, thanks for raising that!

tleyden commented 7 years ago

Aside from the yaml file confusion, where is this supposed to go?

Runtime: python2.7 Handler: handler.Handle

tleyden commented 7 years ago

A little off-topic, but I notice that in this version of the README, you had a TOC, which has later been removed, probably because it was too much work to maintain.

I've recently discovered asciidoctor and would highly recommend taking a look -- it's really not that much harder to use or look at than markdown, and feels much more powerful. If you convert from markdown -> asciidoctor, it will generate TOC's automatically, and github correctly renders asciidoctor -- here's an example repo with an asciidoctor readme.

tleyden commented 7 years ago

Why doesn't the current README have this step mentioned in an earlier version of the docs?

fsenart commented 7 years ago

For the runtime and handler info, when you create a new lambda through the web console of aws or when you use cloudformation you have to fill these parameters. We provide them only for your information in this readme so one knows which parameter to use.

For the TOC I will definitely take a look on the project you've mentioned and will try to come up with a more readable readme :)

fsenart commented 7 years ago

We've removed the configure section and revamped the readme because we've seen two kind of users. Users who have already deployed/used lambda and then do not need creating the basic lambda role and are here to try golang. Users who are totally new to lambda and are here and need a full walkthrough to deploy a golang lambda, for them we now provide the "preview" script which handles all prerequisites. We think it makes the readme less bloated.

tleyden commented 7 years ago

I see. Ok yeah wrapping up some of those steps in a script makes sense.

I guess for this issue, maybe if you just add a note under that "Deploy" section like

When you create a new lambda through the web console of aws or when you use cloudformation you have to fill these parameters

that would make it less confusing, since it would be clear where these parameters are supposed to go.

fsenart commented 7 years ago

@tleyden thank you for your feedback. I've updated the readme in accordance. Feel free if you see other things to improve.