C# run command #488

bryceitoc9 · 2019-04-15T18:40:59Z

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Infrastructure (change which improves the lifecycle management (CI/CD, build, package, deploy, lint, etc) of the application, but does not change functionality.)
Technical debt (change which improves the maintainability of the codebase, but does not change functionality)
Testing (change which modifies or adds test coverage, but does not change functionality)
Documentation (change which modifies or adds documentation, but does not change functionality)

Description

Added implementation for C# Run command through Codelens. This does NOT include any debug work (debug parameters required for run execution have been hardcoded and debug cannot be enabled)

Motivation and Context

We need to be able to run C# Lambdas through SAM Local.

Related Issue(s)

#289

Testing

Manually validated Run command happy paths for dotnet2.0 and dotnet2.1 runtimes. Looking to add further validation tests

Screenshots (if appropriate)

Checklist

I have read the README document
I have read the CONTRIBUTING document
My code follows the code style of this project
I have added tests to cover my changes
All new and existing tests passed
A short description of the change has been added to the CHANGELOG

License

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

src/shared/codelens/csharpCodeLensProvider.ts

src/shared/codelens/localLambdaRunner.ts

bryceitoc9 · 2019-04-15T21:48:01Z

TODO:

Focus on AWS Toolkit Output Channel when starting run (implement this at LocalLambdaRunner stage?)
Unit tests

codecov-io · 2019-04-16T18:51:03Z

Codecov Report

Merging #488 into feature/dotnet will decrease coverage by <.01%.
The diff coverage is 14.7%.

@@                Coverage Diff                 @@
##           feature/dotnet     #488      +/-   ##
==================================================
- Coverage           52.77%   52.76%   -0.01%     
==================================================
  Files                 108      108              
  Lines                4237     4283      +46     
  Branches              624      634      +10     
==================================================
+ Hits                 2236     2260      +24     
- Misses               2001     2023      +22

Impacted Files	Coverage Δ
src/lambda/models/samLambdaRuntime.ts	`27.02% <0%> (ø)`	⬆️
src/shared/codelens/pythonCodeLensProvider.ts	`0% <0%> (ø)`	⬆️
src/shared/codelens/localLambdaRunner.ts	`34.15% <20%> (+3.07%)`	⬆️
src/shared/codelens/csharpCodeLensProvider.ts	`52.08% <9.09%> (-11.08%)`	⬇️
src/shared/utilities/disposableFiles.ts	`94.87% <0%> (-2.57%)`	⬇️
src/shared/sam/cli/samCliBuild.ts	`93.02% <0%> (+20.93%)`	⬆️

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 4360748...712fd03. Read the comment docs.

src/test/shared/codelens/localLambdaRunner.test.ts

mpiroc · 2019-04-16T19:33:40Z

src/shared/codelens/localLambdaRunner.ts

@@ -552,7 +557,9 @@ const getConfig = async (params: {
    return config
 }

-const getEnvironmentVariables = (config: HandlerConfig): SAMTemplateEnvironmentVariables => {
+const getEnvironmentVariables = (


Do we have any tests verifying that:

Environment variables are read from templates.json

Input event is read from templates.json?

If not, we should add them in a separate PR.

Not yet, this was a little scaffolding work for future tests.

bryceitoc9 · 2019-04-16T23:11:05Z

Retrying CodeBuild (and the other builds) worked, but CodeBuild results weren't picked up. Successful build here: https://us-west-2.console.aws.amazon.com/codesuite/codebuild/projects/aws-toolkit-vscode/build/aws-toolkit-vscode%3A35cc138b-5673-4cda-9bee-a7f24cf8d1aa/log?region=us-west-2

bryceitoc9 · 2019-04-16T23:11:20Z

Errors appear to be tied to microsoft/vscode#72446

mpiroc · 2019-04-17T02:19:10Z

FYI the VS Code outage is over, see microsoft/vscode#72438

src/shared/codelens/csharpCodeLensProvider.ts

src/shared/codelens/localLambdaRunner.ts

src/test/shared/codelens/localLambdaRunner.test.ts

awschristou · 2019-04-18T15:39:31Z

src/test/shared/codelens/localLambdaRunner.test.ts

+                channelLogger: new FakeChannelLogger(),
+                // not needed for testing
+                manifestPath: undefined,
+                samProcessInvoker: {


What you do here with samProcessInvoker looks like a great set of tests to add to samCliBuild.test.ts as well.

Will probably save that for later

bryceitoc9 · 2019-04-19T18:07:12Z

attn: @awschristou My latest commit added the ability to run Lambda handlers nested past the .csproj file. Lambda handlers in the following directories are confirmed to work:

src/shared/codelens/localLambdaRunner.ts

src/test/shared/codelens/localLambdaRunner.test.ts

src/shared/codelens/localLambdaRunner.ts

src/lambda/local/detectLocalLambdas.ts

src/lambda/models/samLambdaRuntime.ts

src/shared/codelens/csharpCodeLensProvider.ts

src/shared/codelens/localLambdaRunner.ts

src/shared/codelens/csharpCodeLensProvider.ts

mpiroc · 2019-04-25T22:13:18Z

src/shared/codelens/csharpCodeLensProvider.ts

+                filePath: documentUri.fsPath
+            })
+
+            const relativeFunctionHandler = getRelativeFunctionHandler({


It's not obvious to me what "relative function handler means". How about calling it something like "full handler name" or "fully-qualified handler name"?

The naming convention isn't great but it's what was here before. It's actually a bit of a misnomer too--compiled languages don't have any need for a relative pathing to the handler (if you dig into this function, we return the handler unmodified).

I'll see what I can do.

So on further analysis, when running the Python handler, args.handlerName === relativeOriginalFunctionHandler and handlerName === relativeFunctionHandler (everything is equal in the case where we aren't debugging since the debug run creates a separate handler file for the SAM run).

That being said, this logic might potentially change going forward as we improve our code detection logic for the Python handler (right now, we only pull in the code directory itself, so any dependencies in different directories might not get picked up on SAM build). Let's keep this around a little longer and determine whether or not we need it when we improve that.

mpiroc · 2019-04-25T22:21:44Z

src/shared/codelens/localLambdaRunner.ts

+    codeRoot: string,
+    filePath: string
+}): string {
+    return path.relative(params.codeRoot, path.dirname(params.filePath))


Should this also include the filename without extension? I.e. if the CodeUri is /mycode and filePath is /mycode/src/app.py, we want to resolve to src.app.myHandler, not src.myHandler.

It's not obvious to me what is the intended purpose of either of these functions (getHandlerRelativePath and getRelativeFunctionHandler). What if we replaced them with a single function like:

export function getFullHandlerName({ codeUri, sourceFilePath }: { codeUri: string, sourceFilePath: string }): string { ... }

I'll take a look, most of these weird workflows come from Python (and I'm assuming all other interpreted languages). I'm mainly building these methods and having everything go through the same workflows for continuity between run commands.

mpiroc · 2019-04-25T22:28:30Z

src/shared/codelens/localLambdaRunner.ts

+        case SamLambdaRuntimeFamily.Go:
+        // if the runtime exists but for some reason we forgot to cover it here
+        default:
+            return false


IMO if we forgot to cover the runtime here, we should throw instead of swallowing the error. Otherwise, we may forget to update this function when adding support for a new runtime.

…the handler

bryceitoc9 · 2019-04-26T16:31:30Z

As per discussion with @mpiroc , we can merge this to unblock the debug command work. Wrote an issue to cover the further improvements we should make to this code: #507

…y. (#599) * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

Due to a series of misteps, we are squashing this entire feature branch into develop. 1. We allowed `feature/dotnet` to drift too far from `develop`, which required significant manual conflict resolution to resolve. 2. We should have squashed the conflict-resolution history *before* merging develop, but we did not. Therefore in order to squash the pre-merge commits, we would need to recreate the merge and re-resolve conflicts. This would reduce our confidence in the quality of the branch. In the future: 1. We will regularly merge into feature branches to prevent drift. 2. If we do encounter a situation that involves manually porting, we will squash the ports *before* merging develop. * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

Initial commit for C# run command

1f3c057

bryceitoc9 requested review from awschristou and mpiroc April 15, 2019 18:41

bryceitoc9 commented Apr 15, 2019

View reviewed changes

src/shared/codelens/csharpCodeLensProvider.ts Show resolved Hide resolved

mpiroc suggested changes Apr 15, 2019

View reviewed changes

Responding to feedback

8cdefe6

bryceitoc9 commented Apr 15, 2019

View reviewed changes

src/shared/codelens/csharpCodeLensProvider.ts Outdated Show resolved Hide resolved

bryceitoc9 commented Apr 15, 2019

View reviewed changes

src/shared/codelens/localLambdaRunner.ts Outdated Show resolved Hide resolved

Housekeeping and tests

52cc403

mpiroc reviewed Apr 16, 2019

View reviewed changes

src/test/shared/codelens/localLambdaRunner.test.ts Outdated Show resolved Hide resolved

mpiroc reviewed Apr 16, 2019

View reviewed changes

Picking nits

007e1d9

mpiroc approved these changes Apr 17, 2019

View reviewed changes

bryceitoc9 added 2 commits April 17, 2019 08:56

Updating runtimes to match versions currently supported by SAM CLI

55fda65

Fixed getFamily call to reflect current runtimes

d8a5823

awschristou requested changes Apr 18, 2019

View reviewed changes

bryceitoc9 changed the title ~~C# run command (WIP)~~ C# run command Apr 19, 2019

localLambdaRunner handles nested functions in C#

caee4d8

bryceitoc9 mentioned this pull request Apr 19, 2019

Removing Node 4.3 #499

Merged

12 tasks

mpiroc suggested changes Apr 19, 2019

View reviewed changes

Addressing feedback

7f49004

mpiroc approved these changes Apr 19, 2019

View reviewed changes

bryceitoc9 added 2 commits April 19, 2019 13:54

Fixing test

9c55e1c

Refixing test

86b74d9

awschristou requested changes Apr 22, 2019

View reviewed changes

src/shared/codelens/localLambdaRunner.ts Outdated Show resolved Hide resolved

src/shared/codelens/localLambdaRunner.ts Outdated Show resolved Hide resolved

Broke apart localLambdaRunner.makeInputTemplate call

b8b72ea

awschristou requested changes Apr 23, 2019

View reviewed changes

Addressing PR feedback

9ea83c1

awschristou approved these changes Apr 24, 2019

View reviewed changes

mpiroc reviewed Apr 25, 2019

View reviewed changes

Throwing error if runtime doesn't have its relative path appended to …

712fd03

…the handler

awschristou approved these changes Apr 26, 2019

View reviewed changes

bryceitoc9 mentioned this pull request Apr 26, 2019

Consolidate "local lambda runner" implementations. #507

Closed

bryceitoc9 merged commit 7b69458 into feature/dotnet Apr 26, 2019

bryceitoc9 deleted the bryceito/dotnet/run branch April 26, 2019 17:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C# run command #488

C# run command #488

bryceitoc9 commented Apr 15, 2019

bryceitoc9 commented Apr 15, 2019 •

edited

Loading

codecov-io commented Apr 16, 2019 •

edited

Loading

mpiroc Apr 16, 2019

bryceitoc9 Apr 16, 2019

bryceitoc9 commented Apr 16, 2019

bryceitoc9 commented Apr 16, 2019

mpiroc commented Apr 17, 2019

awschristou Apr 18, 2019

bryceitoc9 Apr 19, 2019

bryceitoc9 commented Apr 19, 2019

mpiroc Apr 25, 2019

bryceitoc9 Apr 25, 2019

bryceitoc9 Apr 26, 2019

mpiroc Apr 25, 2019

mpiroc Apr 25, 2019

bryceitoc9 Apr 25, 2019

mpiroc Apr 25, 2019

bryceitoc9 Apr 26, 2019

bryceitoc9 commented Apr 26, 2019

C# run command #488

C# run command #488

Conversation

bryceitoc9 commented Apr 15, 2019

Types of changes

Description

Motivation and Context

Related Issue(s)

Testing

Screenshots (if appropriate)

Checklist

License

bryceitoc9 commented Apr 15, 2019 • edited Loading

codecov-io commented Apr 16, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryceitoc9 commented Apr 16, 2019

bryceitoc9 commented Apr 16, 2019

mpiroc commented Apr 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryceitoc9 commented Apr 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryceitoc9 commented Apr 26, 2019

bryceitoc9 commented Apr 15, 2019 •

edited

Loading

codecov-io commented Apr 16, 2019 •

edited

Loading