Fix exceptions thrown in a run config breaking the Run Config action … #896

abrooksv · 2019-04-08T22:26:59Z

…button

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Description

Resolve some TODOs to simplify the LambdaBuilder logic
Make sure all exceptions get mapped into CompletableFuture.completeExceptionally

Motivation and Context

Related Issue(s)

#868

Testing

Forced exceptions to be thrown, action button still works after

Screenshots (if appropriate)

Checklist

I have read the README document
I have read the CONTRIBUTING document
Local run of gradlew check succeeds
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

I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-io · 2019-04-08T23:26:46Z

Codecov Report

Merging #896 into master will decrease coverage by 16.54%.
The diff coverage is 14.28%.

@@              Coverage Diff              @@
##             master     #896       +/-   ##
=============================================
- Coverage     60.93%   44.38%   -16.55%     
+ Complexity      677      509      -168     
=============================================
  Files           166      166               
  Lines          5503     5499        -4     
  Branches        700      699        -1     
=============================================
- Hits           3353     2441      -912     
- Misses         1848     2838      +990     
+ Partials        302      220       -82

Flag	Coverage Δ	Complexity Δ
#integtest	`?`	`?`
#uitest	`?`	`?`
#unittest	`44.38% <14.28%> (-0.15%)`	`509 <1> (-1)`

Impacted Files	Coverage Δ	Complexity Δ
...oolkits/jetbrains/services/lambda/LambdaBuilder.kt	`9.27% <0%> (-69.67%)`	`2 <0> (-7)`
...ts/jetbrains/services/lambda/LambdaBuilderUtils.kt	`9.09% <19.23%> (-63.14%)`	`2 <1> (-4)`
...e/src/software/aws/toolkits/core/s3/BucketUtils.kt	`0% <0%> (-100%)`	`0% <0%> (ø)`
.../jetbrains/services/telemetry/MessageBusService.kt	`0% <0%> (-100%)`	`0% <0%> (ø)`
...rains/services/lambda/execution/local/SamRunner.kt	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...tials/ExtensionPointCredentialsProviderRegistry.kt	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...ns/services/lambda/python/PythonSamDebugSupport.kt	`0% <0%> (-96.97%)`	`0% <0%> (-3%)`
...s/jetbrains/ui/wizard/IntelliJSdkSelectionPanel.kt	`0% <0%> (-88.89%)`	`0% <0%> (-6%)`
...kits/jetbrains/services/lambda/SamProjectWizard.kt	`0% <0%> (-83.64%)`	`0% <0%> (ø)`
...etbrains/services/lambda/deploy/SamDeployDialog.kt	`0% <0%> (-82.31%)`	`0% <0%> (-16%)`
... and 65 more

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 175eb8f...dcc141c. Read the comment docs.

kiiadi · 2019-04-11T15:33:55Z

jetbrains-core/src/software/aws/toolkits/jetbrains/services/lambda/LambdaBuilder.kt

@@ -133,7 +133,11 @@ abstract class LambdaBuilder {
            processHandler.startNotify()
        }

-        return future
+        return try {
+            future.get()


Why are we moving this to blocking now? Future.get is an anti-pattern!

Its used in an async method in LambdaBuilderUtils, which has to have this call block, else the progress bar immediately goes away

https://github.com/aws/aws-toolkit-jetbrains/pull/896/files#diff-af40ca3afd894931a84a159370b1357fR91

Discussed offline, we need this method to block till underlying sam process is done. Removed the unneeded run on BG thread call though

abrooksv · 2019-06-24T22:47:39Z

@aws/aws-ides-team Any other concerns?

zhangzhx · 2019-06-25T21:46:45Z

LGTM. We have the LambdaBuilder file refactored a bit, need to resolve conflicts 😬

…button * Resolve some TODOs to simplify the LambdaBuilder logic * Make sure all exceptions get mapped into CompletableFuture.completeExceptionally

…button (#896) * Resolve some TODOs to simplify the LambdaBuilder logic * Make sure all exceptions get mapped into CompletableFuture.completeExceptionally

* Switch out our test logger (#1027) * Fixing alternative IDE logic so that it runs for every runTask defined in the project. (#1029) * Refactor New Project Wizard to extensions to support other platforms (#1022) * Fix the bug when project is not located in the same drive of temp fol… (#1034) * Fix the refresh vfs in ReadAction bug. (#1060) * Fix the refresh vfs in ReadAction bug. * Use sam build folder to avoid recursive copying. * Fix exceptions thrown in a run config breaking the Run Config action button (#896) * Resolve some TODOs to simplify the LambdaBuilder logic * Make sure all exceptions get mapped into CompletableFuture.completeExceptionally * Enable using the presence of a requirements.txt to determine if a met… (#1056) * Enable using the presence of a requirements.txt to determine if a method is a handler in Python * This allows us to not rely on source roots but instead matches what SAM build will do, which is look for the requirements.txt at the codeUri location * Fix broken test that had incorrect assertion and behavior * Fix tests not testing the correct things * Fix dynamodb template for python not creating successfully. (#1071) * Remove anonymized. (#1074) * Only build the specified lambda function when running/debugging from (#1075) * Remove the BaseState system from run configs (#1073) * It kept having aliasing issues related to editing a cloned run config * It kept us from 192 due to NoSuchMethod error * Fix exceptions while generating projects and on run config validation (#1077) - Fix exception on startup while gathering SAM CLI information; replace synchronous execution on EDT with modifying project model after project is initialized - Fix validation for template.yaml file to be able to call SAM deploy action - Fix sync calls on run configuration validation to use async calls - Add tests to verify SamVersion evaluation and getFileInfo logic * Fix LocalLambdaRunConfiguration tests broken by async SAM version check (#1079) * Move test back to unit test package * Make FileInfoCache async based * Move logic up from SAM version cache to FileInfoCache * Tweak the evaluateBlocking to throw if not successful * Move tests around and update to match the current state of the world * Switch UI tests to use a regex for SDK selection (#1084) * This allows tests to run with Java 11 or Java 8 * Fix Virtual File System refresh after new project is generated (#1083) * Refresh Vfs after project is updated from SAM CLI to sync it with File System * WriteAction is not necessary while SAM project is generated since there is no interaction with VFS. We refresh VFS after files are copied. * Re-enable 192 EAP (#1076) * Revert "Revert "Enable toolkit in 192 (#1000)" (#1007)" This reverts commit fcdfa2c. * Remove warning around state variable name * Make sure Java lambdas tests compile as 1.8 * Fix incompatible plugin versions * Suppress and comment deprecation * Fix SAM SemVer tests since now 192 cares about things after the patch version * Fix toString() on SDK choices in UI test framework (#1085) * Fix Java UI tests on 2018.3 (#1086) * Fix read/write action errors when invoking a SAM lambda (#1087) * Fix Windows SAM detection tests not running on Windows (#1089) * Push coverage to CodeCov * Add test artifact saving for linux-based CI jobs (#1088) * Add test artifact saving for linux-based CI jobs * Backfill credential management document (#1055) * Break up the AwsExplorerNodes (#1091) * No functional changes were made * Remove unused code (#1093) * Wrap credential retrieving with progress indication if on UI thread (#1096) * This pushes the network call itself to a background thread while showing a progress bar that we are doing something. This is what is suggested by https://github.com/JetBrains/intellij-community/blob/62d0e1a5010fef3369d91b670a3cc7b11ac73ba1/platform/platform-api/src/com/intellij/execution/process/OSProcessHandler.java#L105 * Remove the endpoints.json URL that does not use HTTPS (#1097) * Add changelog for #1096 (#1098) * Pull in latest AWS SDK to update Lambda models (#1100) * Simplify explorer service node (#1092) * Rename some of the local and remote lambda class names to be more consistent. (#1101) * Update to stable 192 versions, fix UI tests (#1106) * Fix the ui tests since 192 changes JBTabs to a brand new system * Remove LambdaHandlerIndex (#1107) * In 192, we were showing that we were triggering reentrant indexing. Since we only use this for Java handler completion, we will remove it. * Updating version to 1.5 * Updating SNAPSHOT version to 1.6-SNAPSHOT * Fix Create Lambda action missing from explorer menu (#1111) * Implement resource cache - to be used for caching describe calls. (#1102) * Support Gradle link (#1080) * Add library test in Gradle UI test and change the importing behavior to background async. * Fork JetBrains' AsyncTreeModel to remove the read action requirement (#1110) * JetBrain's Invoker is forced to run under a read action, fork the Invoker and AsyncTreeModel (and supporting classes) to remove that requirement * Increase min version to drop 183 EAP builds, implement abstract method that wasnt implemented in earlier versions * Add IU 192 profile

abrooksv requested a review from a team as a code owner April 8, 2019 22:27

abrooksv force-pushed the fixUncaughtErrorsInRunConfig branch from e5a01f2 to 53a51d7 Compare April 8, 2019 22:59

kiiadi reviewed Apr 11, 2019

View reviewed changes

abrooksv force-pushed the fixUncaughtErrorsInRunConfig branch 2 times, most recently from 98e6f5f to 05b6f7c Compare April 12, 2019 22:38

abrooksv mentioned this pull request Jun 20, 2019

Cancelling an MFA prompt triggers an uncaught IDE error #1050

Closed

zhangzhx approved these changes Jun 25, 2019

View reviewed changes

abrooksv force-pushed the fixUncaughtErrorsInRunConfig branch from 38b1c9c to d44e1a4 Compare June 26, 2019 20:45

Fix exceptions thrown in a run config breaking the Run Config action …

dcc141c

…button * Resolve some TODOs to simplify the LambdaBuilder logic * Make sure all exceptions get mapped into CompletableFuture.completeExceptionally

abrooksv force-pushed the fixUncaughtErrorsInRunConfig branch from d44e1a4 to dcc141c Compare June 26, 2019 20:47

zhangzhx approved these changes Jun 26, 2019

View reviewed changes

abrooksv merged commit 164ec2e into master Jun 26, 2019

abrooksv deleted the fixUncaughtErrorsInRunConfig branch June 26, 2019 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix exceptions thrown in a run config breaking the Run Config action … #896

Fix exceptions thrown in a run config breaking the Run Config action … #896

abrooksv commented Apr 8, 2019

codecov-io commented Apr 8, 2019 •

edited

Loading

kiiadi Apr 11, 2019

abrooksv Apr 11, 2019

abrooksv Apr 11, 2019

abrooksv Apr 12, 2019

abrooksv commented Jun 24, 2019

zhangzhx commented Jun 25, 2019

Fix exceptions thrown in a run config breaking the Run Config action … #896

Fix exceptions thrown in a run config breaking the Run Config action … #896

Conversation

abrooksv commented Apr 8, 2019

Types of changes

Description

Motivation and Context

Related Issue(s)

Testing

Screenshots (if appropriate)

Checklist

License

codecov-io commented Apr 8, 2019 • edited Loading

Codecov Report

kiiadi Apr 11, 2019

Choose a reason for hiding this comment

abrooksv Apr 11, 2019

Choose a reason for hiding this comment

abrooksv Apr 11, 2019

Choose a reason for hiding this comment

abrooksv Apr 12, 2019

Choose a reason for hiding this comment

abrooksv commented Jun 24, 2019

zhangzhx commented Jun 25, 2019

codecov-io commented Apr 8, 2019 •

edited

Loading