Post startup script notifications extension for Code Editor #109

astitv-sh · 2025-03-25T21:55:05Z

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.

bharathGuntamadugu

We also need this extensions to be part of vscode/build/npm/dirs.js to install the depencies correctly? Ref: #110

bharathGuntamadugu · 2025-03-26T16:53:22Z

patched-vscode/extensions/post-startup-notifications/.vscode-test.mjs

Curious why do we need this?

bharathGuntamadugu · 2025-03-26T17:17:04Z

patched-vscode/extensions/post-startup-notifications/.vscode/launch.json

+// Use IntelliSense to learn about possible attributes.
+// Hover to view descriptions of existing attributes.
+// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+{


Spacing seems off with more than tab spaces for all jsons.

Fixed locally, not sure why shows up here.

bharathGuntamadugu · 2025-03-26T17:21:25Z

patched-vscode/extensions/post-startup-notifications/package.json

@@ -0,0 +1,42 @@
+{


See if we need to add publisher and license in this file? Have seen this in other extension (theme) we build

"publisher": "sagemaker", "license": "MIT",

bharathGuntamadugu · 2025-03-26T17:22:15Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+let previousStatus: string | undefined;
+export function activate(context: vscode.ExtensionContext) {
+    try {
+		const watcher = chokidar.watch(STATUS_FILE, {


Spacing seems off here.

bharathGuntamadugu · 2025-03-26T17:33:23Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+import * as chokidar from 'chokidar';
+
+let previousStatus: string | undefined;
+export function activate(context: vscode.ExtensionContext) {


We are only suppose to activate this SMUS and not for SM AI.

bharathGuntamadugu · 2025-03-26T17:34:11Z

patched-vscode/extensions/post-startup-notifications/src/constant.ts

@@ -0,0 +1 @@
+export const STATUS_FILE = '/tmp/.post-startup-status.json';


nit: STATUS_FILE -> POST_START_UP_STATUS_FILE

bharathGuntamadugu · 2025-03-26T17:35:48Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+import * as fs from 'fs';
+import { STATUS_FILE } from './constant';
+import { StatusFile } from './types';
+import * as chokidar from 'chokidar';


Whats with chokidar package? Is this approved to be used?

Yes, it is approved for use. It is also available internally as part of the internal package registry.

bharathGuntamadugu · 2025-03-26T17:38:07Z

patched-vscode/extensions/post-startup-notifications/package.json

+    "vscode:prepublish": "npm run build-ext",
+    "build-ext": "node ../../node_modules/gulp/bin/gulp.js --gulpfile ../../build/gulpfile.extensions.js compile-extension:post-startup-notifications ./tsconfig.json"
+  },
+  "devDependencies": {


As we have dependencies here - my understanding is we need to have yarn.lock file to install these dependencies.

bharathGuntamadugu · 2025-03-27T00:12:32Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+
+let previousStatus: string | undefined;
+export function activate(context: vscode.ExtensionContext) {
+    try {


What happens if the file does not exits?

The extension monitors the path and handles both cases for when file is first created as well as changed

aws-asolidu · 2025-03-27T18:21:40Z

Shouldn't this be part of the post-startup script instead of it's own extension? #108

aws-asolidu · 2025-03-27T18:30:39Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+					case 'in-progress':
+						vscode.window.showInformationMessage(statusData.message);
+						break;
+					default:
+						vscode.window.showInformationMessage(statusData.message);


seems like in-progress case is not necessary.

In progress case is handled as per the equivalent plugin for JL. Can remove if my info is not up to date.

it has same logic as default case.

+1

can we say like to be more redable:

case 'in-progress': default: vscode.window.showInformationMessage(statusData.message);

aws-asolidu · 2025-03-27T18:32:37Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+let previousStatus: string | undefined;
+export function activate(context: vscode.ExtensionContext) {
+    try {
+		const watcher = chokidar.watch(STATUS_FILE, {


we should close the watcher on extension deactivate.

aws-asolidu · 2025-03-27T18:38:54Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+};
+
+export function deactivate() {
+    console.log('Status monitor deactivated');


can we use outputchannel logging instead of console logs? https://github.com/aws/sagemaker-code-editor/pull/110/files#diff-2f5d009765a1a962b8260a8acaebfbc6b170c022c76407605985311b69d35afeR18

astitv-sh · 2025-03-27T18:50:54Z

Shouldn't this be part of the post-startup script instead of it's own extension? #108

Post Startup writes the execution status logs to the post-startup-status.json log file. This extension monitors the log file to notify the user in real time.

aws-asolidu · 2025-03-27T18:53:24Z

Shouldn't this be part of the post-startup script instead of it's own extension? #108

Post Startup writes the execution status logs to the post-startup-status.json log file. This extension monitors the log file to notify the user in real time.

Why do we need a separate extension for each post-startup use-case?

bharathGuntamadugu · 2025-03-28T03:31:06Z

patched-vscode/extensions/post-startup-notifications/__mocks__/vscode.js

Do we need this file? Its empty.

bharathGuntamadugu · 2025-03-28T03:32:57Z

patched-vscode/extensions/post-startup-notifications/src/constant.ts

@@ -0,0 +1,3 @@
+export const POST_START_UP_STATUS_FILE = '/tmp/.post-startup-status.json';
+export const SAGEMAKER_UNIFIED_STUDIO_ENV_VALUE = 'SageMakerUnifiedStudio';
+export const SAGEMAKER_UNIFIED_STUDIO_ENV_KEY = 'SERVICE_NAME';


nit: Wld rather name it SERVICE_NAME_ENV_KEY

bharathGuntamadugu · 2025-03-28T03:34:15Z

patched-vscode/extensions/post-startup-notifications/src/extension.ts

+					case 'in-progress':
+						vscode.window.showInformationMessage(statusData.message);
+						break;
+					default:
+						vscode.window.showInformationMessage(statusData.message);


+1

can we say like to be more redable:

case 'in-progress': default: vscode.window.showInformationMessage(statusData.message);

bharathGuntamadugu · 2025-03-28T03:35:34Z

patched-vscode/extensions/post-startup-notifications/yarn.lock

@@ -0,0 +1,2 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.


You need to run yarn install from this folder to generate pacakge versions in the yarn.lock file

kjayan · 2025-03-28T05:26:36Z

patched-vscode/extensions/post-startup-notifications/webpack.config.js

+/** @type WebpackConfig */
+const extensionConfig = {
+  target: 'node', // VS Code extensions run in a Node.js-context 📖 -> https://webpack.js.org/configuration/node/
+	mode: 'none', // this leaves the source code as close as possible to the original (when packaging we set this to 'production')


Nit: this spacing issue appears only in the PR or appears locally too?

bharathGuntamadugu reviewed Mar 26, 2025

View reviewed changes

bharathGuntamadugu reviewed Mar 27, 2025

View reviewed changes

aws-asolidu reviewed Mar 27, 2025

View reviewed changes

bharathGuntamadugu suggested changes Mar 28, 2025

View reviewed changes

kjayan reviewed Mar 28, 2025

View reviewed changes

astitv-sh closed this Mar 28, 2025

astitv-sh force-pushed the feature/smus branch from d6d790b to c9e64a3 Compare March 28, 2025 13:37

astitv-sh mentioned this pull request Mar 28, 2025

Post startup script notifications extension for Code Editor #113

Merged

		@@ -0,0 +1 @@
		export const STATUS_FILE = '/tmp/.post-startup-status.json'; No newline at end of file

		@@ -0,0 +1,2 @@
		# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

Post startup script notifications extension for Code Editor #109

Post startup script notifications extension for Code Editor #109

Uh oh!

Conversation

astitv-sh commented Mar 25, 2025

Uh oh!

bharathGuntamadugu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aws-asolidu commented Mar 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astitv-sh commented Mar 27, 2025

Uh oh!

aws-asolidu commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aws-asolidu commented Mar 27, 2025 •

edited

Loading