Post startup script notifications extension for Code Editor #113
Conversation
bharathGuntamadugu
left a comment
bharathGuntamadugu
left a comment
There was a problem hiding this comment.
Empty file at patched-vscode/extensions/post-startup-notifications/mocks/vscode.js?
| // cwd: extensionsPath, | ||
| // ignore: ['**/out/**', '**/node_modules/**'] | ||
| // }); | ||
| const compilations = [ |
There was a problem hiding this comment.
Whats with this spacing changes? Automated changes? If not - lets not disturb this.
There was a problem hiding this comment.
Fixed. It shows the entire diff due to comparison from previous commit but the lines are correctly spaced now
| const fs = require('fs'); | ||
|
|
||
| // Complete list of directories where yarn should be executed to install node modules | ||
| const dirs = [ |
There was a problem hiding this comment.
Confusing to understand what is added and what is not added here.
There was a problem hiding this comment.
Only the lines in dirs.js and gulpfile.extensions.js containing post-startup-notifications has been added.
| @@ -0,0 +1,17 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "module": "Node16", | |||
There was a problem hiding this comment.
How do we ensure to have Node16 here? How easy is it to udpate it? Since this is an extension running in client browser - curious what is node16 is not supported?
There was a problem hiding this comment.
These config options are only required during compilation steps, which will be utilized as part of the code editor build process, before it runs on the client end
|
Do we have an answer for this #109 (comment) |
This is required to import a mock version of the vscode module dependency to run the unit tests. |
The post startup script changes added as part of earlier PR add api endpoint. The purpose of this extension is to surface and notify any errors to users during post startup script execution. This requires the changes on code editor end. |
| @@ -0,0 +1,3 @@ | |||
| export const POST_START_UP_STATUS_FILE = '/tmp/.post-startup-status.json'; | |||
| export const SERVICE_NAME_ENV_VALUE = 'SageMakerUnifiedStudio'; | |||
There was a problem hiding this comment.
nit: SERVICE_NAME_ENV_VALUE -> SAGEMAKER_UNIFIED_STUDIO
Post startup script notifications extension for Code Editor
3 participants
Issue #, if available:
Description of changes:
When the post startup script is executed, it can result in errors due to multiple reasons. At present, there is no functionality to surface these failure/success scenarios to inform the users and prompt them to take the necessary action.
This extension monitors the post-startup-status.json file generated during the course of the post startup script execution and shows pop-up notifications to users based on the status of the execution. The possible status set can be ['in-progress','error','success']. In case of errors, the potential cause is also written to the post-startup-status.json file which can be surfaced to the user.
Related post startup script changes PR: https://github.com/aws/private-sagemaker-distribution-staging/pull/237
Testing
The extension was tested for the different cases in Code Editor by manually executing the post startup script which writes to the post-startup-status.json file throughout its workflow. Based on the different test cases, the toast notification shown and the contents of the notification are as follows:
Success case:
Error case (domain metadata fetch issue):
Unexpected error:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.