Skip to content

Conversation

@BrianFarnhill
Copy link

Description of changes:

Added support for PowerShell lambda functions. This is my first PR here so let me know if I missed anything!

Checklist:

  • Write unit tests
  • Write/update functional tests
  • Write/update integration tests
  • make pr passes
  • Write documentation

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

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

A couple of files to be updated according to the new DotNet sample (being updated in a separate PR) and some minor points to make sure we have a standard simple quickstart for customers.

Thank you so much Brian for taking this to a spin and more importantly congrats to your first PR!!!

@@ -0,0 +1,33 @@
AWSTemplateFormatVersion: '2010-09-09'
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update this template.yaml following this new one for dotnet?

https://github.com/awslabs/aws-sam-cli/blob/0622b9bb9b601a6c8c1e4054c2a0c93c2e70c96b/samcli/local/init/templates/cookiecutter-aws-sam-hello-dotnet/%7B%7Bcookiecutter.project_name%7D%7D/template.yaml

Essentially, it removes Env Variables (not being used) and adds additional comments in the Outputs

Copy link
Author

Choose a reason for hiding this comment

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

Looking at the new file, I don't think I need to change this as I don't include the API in this example (the 50 second warm up time based on module loading makes that prohibitive), let me know if you tihnk anything specific is missing from my file though and ill change it.

Properties:
CodeUri: ./artifacts/Function.zip
Handler: Function::Function.Bootstrap::ExecuteFunction
Runtime: dotnetcore2.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is powershell only supported in dotnetcore2.1? Do we want to hardcode that or have some logic like:

            {%- if cookiecutter.runtime == 'dotnetcore2.0' %}
            Runtime: {{ cookiecutter.runtime }}
            {%- elif cookiecutter.runtime == 'dotnetcore2.1' or cookiecutter.runtime == 'dotnet' %}
            Runtime: dotnetcore2.1
            {%- endif %}

That will be parsed at runtime when the customer pass in the flag --runtime <value>

Copy link
Author

Choose a reason for hiding this comment

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

It only works on 2.1 so I hard coded that in, it won't work on 2.0

Variables:
PARAM1: VALUE

Outputs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is PowerShell not support API Gateway Proxy Integration mode?

If it does, would you be able to update the template.yaml to so ``sam local start-api` would work? It's missing the Events and the Outputs section.

Copy link
Author

Choose a reason for hiding this comment

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

It does - but as mentioned above the warm up time on the functions means it usually wouldn't be a use case that most people adopt, so I took it out. Happy to put it in if needed.

@sriram-mv sriram-mv self-assigned this Feb 19, 2019
@sriram-mv sriram-mv self-requested a review February 19, 2019 20:10
### PowerShell (all Operating Systems)

```powershell
build.ps1
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this require any builder changes to aws-lambda-builders, and additions to sam build?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a current .NET builder in the works at aws-lambda-builders, do take a look, if this can take advantage of that.

aws/aws-lambda-builders#80

* [.NET Core installed](https://www.microsoft.com/net/download)
* [PowerShell Core installed](https://docs.microsoft.com/en-us/powershell/scripting/setup/installing-powershell?view=powershell-6)
* [AWSLambdaPSCore PowerShell module](https://www.powershellgallery.com/packages/AWSLambdaPSCore/1.1.0.0)
* [Pester PowerShell module](https://github.com/pester/Pester)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Pester the most used test framework? We need to start customers off on the right path.

@jfuss jfuss added area/init sam init type/feature Feature request labels Nov 12, 2019
@jfuss
Copy link
Contributor

jfuss commented Apr 15, 2020

We still see value in adding powershell in sam init. Instead of adding them directly in here, we should add them into https://github.com/awslabs/aws-sam-cli-app-templates/ instead. This will allow us to define a powershell example within the .NetCore runtime.

Closing this PR but we would love a PR for this to our app templates repo that SAM CLI uses for init.

@jfuss jfuss closed this Apr 15, 2020
BrianFarnhill added a commit to BrianFarnhill/aws-sam-cli-app-templates that referenced this pull request Sep 17, 2020
Took the code I wrote for an old PR in the aws-sam-cli repo
(aws/aws-sam-cli#820) and freshened it up for
this templates library. I have put them under .net as that's the runtime
as far as Lambda is concerned, and have had to use the makefile build
approach to get it to properly build out the dependencies (since letting
the default dotnet one run wouldn't yield any results in this case).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/init sam init type/feature Feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants