-
Notifications
You must be signed in to change notification settings - Fork 736
feat(smus): UX improvements for connecting running spaces from toolkit #8261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Will-ShaoHua
merged 1 commit into
aws:master
from
bhavya2109sharma:feature/smus-ux-improvements
Nov 7, 2025
+692
−90
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,10 +16,19 @@ import _ from 'lodash' | |
| import { prepareDevEnvConnection, tryRemoteConnection } from './model' | ||
| import { ExtContext } from '../../shared/extensions' | ||
| import { SagemakerClient } from '../../shared/clients/sagemaker' | ||
| import { AccessDeniedException } from '@amzn/sagemaker-client' | ||
| import { ToolkitError } from '../../shared/errors' | ||
| import { showConfirmationMessage } from '../../shared/utilities/messages' | ||
| import { RemoteSessionError } from '../../shared/remoteSession' | ||
| import { ConnectFromRemoteWorkspaceMessage, InstanceTypeError } from './constants' | ||
| import { | ||
| ConnectFromRemoteWorkspaceMessage, | ||
| InstanceTypeError, | ||
| InstanceTypeInsufficientMemory, | ||
| InstanceTypeInsufficientMemoryMessage, | ||
| RemoteAccess, | ||
| RemoteAccessRequiredMessage, | ||
| SpaceStatus, | ||
| } from './constants' | ||
| import { SagemakerUnifiedStudioSpaceNode } from '../../sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioSpaceNode' | ||
|
|
||
| const localize = nls.loadMessageBundle() | ||
|
|
@@ -136,10 +145,10 @@ export async function stopSpace( | |
| sageMakerClient?: SagemakerClient | ||
| ) { | ||
| await tryRefreshNode(node) | ||
| if (node.getStatus() === 'Stopped' || node.getStatus() === 'Stopping') { | ||
| if (node.getStatus() === SpaceStatus.STOPPED || node.getStatus() === SpaceStatus.STOPPING) { | ||
| void vscode.window.showWarningMessage(`Space ${node.spaceApp.SpaceName} is already in Stopped/Stopping state.`) | ||
| return | ||
| } else if (node.getStatus() === 'Starting') { | ||
| } else if (node.getStatus() === SpaceStatus.STARTING) { | ||
| void vscode.window.showWarningMessage( | ||
| `Space ${node.spaceApp.SpaceName} is in Starting state. Wait until it is Running to attempt stop again.` | ||
| ) | ||
|
|
@@ -167,7 +176,7 @@ export async function stopSpace( | |
| }) | ||
| } catch (err) { | ||
| const error = err as Error | ||
| if (error.name === 'AccessDeniedException') { | ||
| if (error instanceof AccessDeniedException) { | ||
| throw new ToolkitError('You do not have permission to stop spaces. Please contact your administrator', { | ||
| cause: error, | ||
| code: error.name, | ||
|
|
@@ -195,56 +204,205 @@ export async function openRemoteConnect( | |
| const spaceName = node.spaceApp.SpaceName! | ||
| await tryRefreshNode(node) | ||
|
|
||
| // for Stopped SM spaces - check instance type before showing progress | ||
| if (node.getStatus() === 'Stopped') { | ||
| // In case of SMUS, we pass in a SM Client and for SM AI, it creates a new SM Client. | ||
| const client = sageMakerClient ? sageMakerClient : new SagemakerClient(node.regionCode) | ||
|
|
||
| try { | ||
| await client.startSpace(spaceName, node.spaceApp.DomainId!) | ||
| await tryRefreshNode(node) | ||
| const appType = node.spaceApp.SpaceSettingsSummary?.AppType | ||
| if (!appType) { | ||
| throw new ToolkitError('AppType is undefined for the selected space. Cannot start remote connection.', { | ||
| code: 'undefinedAppType', | ||
| }) | ||
| const remoteAccess = node.spaceApp.SpaceSettingsSummary?.RemoteAccess | ||
| const nodeStatus = node.getStatus() | ||
|
|
||
| // Route to appropriate handler based on space state | ||
| if (nodeStatus === SpaceStatus.RUNNING && remoteAccess !== RemoteAccess.ENABLED) { | ||
| return handleRunningSpaceWithDisabledAccess(node, ctx, spaceName, sageMakerClient) | ||
| } else if (nodeStatus === SpaceStatus.STOPPED) { | ||
| return handleStoppedSpace(node, ctx, spaceName, sageMakerClient) | ||
| } else if (nodeStatus === SpaceStatus.RUNNING) { | ||
| return handleRunningSpaceWithEnabledAccess(node, ctx, spaceName) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an instance type upgrade will be needed for remote access | ||
| */ | ||
| export async function checkInstanceTypeUpgradeNeeded( | ||
| node: SagemakerSpaceNode | SagemakerUnifiedStudioSpaceNode, | ||
| sageMakerClient?: SagemakerClient | ||
| ): Promise<{ upgradeNeeded: boolean; currentType?: string; recommendedType?: string }> { | ||
| const client = sageMakerClient || new SagemakerClient(node.regionCode) | ||
|
|
||
| try { | ||
| const spaceDetails = await client.describeSpace({ | ||
| DomainId: node.spaceApp.DomainId!, | ||
| SpaceName: node.spaceApp.SpaceName!, | ||
| }) | ||
|
|
||
| const appType = spaceDetails.SpaceSettings!.AppType! | ||
|
|
||
| // Get current instance type | ||
| const currentResourceSpec = | ||
| appType === 'JupyterLab' | ||
| ? spaceDetails.SpaceSettings!.JupyterLabAppSettings?.DefaultResourceSpec | ||
| : spaceDetails.SpaceSettings!.CodeEditorAppSettings?.DefaultResourceSpec | ||
|
|
||
| const currentInstanceType = currentResourceSpec?.InstanceType | ||
|
|
||
| // Check if upgrade is needed | ||
| if (currentInstanceType && currentInstanceType in InstanceTypeInsufficientMemory) { | ||
bhavya2109sharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Current type has insufficient memory | ||
| return { | ||
| upgradeNeeded: true, | ||
| currentType: currentInstanceType, | ||
| recommendedType: InstanceTypeInsufficientMemory[currentInstanceType], | ||
| } | ||
| } | ||
|
|
||
| return { upgradeNeeded: false, currentType: currentInstanceType } | ||
| } catch (err) { | ||
| const error = err as Error | ||
| if (error instanceof AccessDeniedException) { | ||
| throw new ToolkitError('You do not have permission to describe spaces. Please contact your administrator', { | ||
bhavya2109sharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| cause: error, | ||
| code: error.name, | ||
| }) | ||
| } | ||
| throw err | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Handles connecting to a running space with disabled remote access | ||
| * Requires stopping the space, enabling remote access, and restarting | ||
| */ | ||
| async function handleRunningSpaceWithDisabledAccess( | ||
bhavya2109sharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| node: SagemakerSpaceNode | SagemakerUnifiedStudioSpaceNode, | ||
| ctx: vscode.ExtensionContext, | ||
| spaceName: string, | ||
| sageMakerClient?: SagemakerClient | ||
| ) { | ||
| // Check if instance type upgrade will be needed | ||
| const instanceTypeInfo = await checkInstanceTypeUpgradeNeeded(node, sageMakerClient) | ||
|
|
||
| let prompt: string | ||
| if (instanceTypeInfo.upgradeNeeded) { | ||
| prompt = InstanceTypeInsufficientMemoryMessage( | ||
| spaceName, | ||
| instanceTypeInfo.currentType!, | ||
| instanceTypeInfo.recommendedType! | ||
| ) | ||
| } else { | ||
| // Only remote access needs to be enabled | ||
| prompt = RemoteAccessRequiredMessage | ||
| } | ||
|
|
||
| const confirmed = await showConfirmationMessage({ | ||
| prompt, | ||
| confirm: 'Restart and Connect', | ||
| cancel: 'Cancel', | ||
| type: 'warning', | ||
| }) | ||
|
|
||
| if (!confirmed) { | ||
| return | ||
| } | ||
bhavya2109sharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Enable remote access and connect | ||
| const client = sageMakerClient || new SagemakerClient(node.regionCode) | ||
|
|
||
| return await vscode.window.withProgress( | ||
| { | ||
| location: vscode.ProgressLocation.Notification, | ||
| cancellable: false, | ||
| title: `Connecting to ${spaceName}`, | ||
| }, | ||
| async (progress) => { | ||
| try { | ||
| // Show initial progress message | ||
| progress.report({ message: 'Stopping the space' }) | ||
|
|
||
| // Stop the running space | ||
| await client.deleteApp({ | ||
| DomainId: node.spaceApp.DomainId!, | ||
| SpaceName: spaceName, | ||
| AppType: node.spaceApp.App!.AppType!, | ||
| AppName: node.spaceApp.App?.AppName, | ||
| }) | ||
|
|
||
| // Only start showing progress after instance type validation | ||
| return await vscode.window.withProgress( | ||
| { | ||
| location: vscode.ProgressLocation.Notification, | ||
| cancellable: false, | ||
| title: `Connecting to ${spaceName}`, | ||
| }, | ||
| async (progress) => { | ||
| progress.report({ message: 'Starting the space.' }) | ||
| await client.waitForAppInService(node.spaceApp.DomainId!, spaceName, appType) | ||
| await tryRemoteConnection(node, ctx, progress) | ||
| // Update progress message | ||
| progress.report({ message: 'Starting the space' }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit : Can maybe include the new instanceType in the case we are also upgrading instance type. |
||
|
|
||
| // Start the space with remote access enabled (skip prompts since user already consented) | ||
| await client.startSpace(spaceName, node.spaceApp.DomainId!, true) | ||
| await tryRefreshNode(node) | ||
| await client.waitForAppInService(node.spaceApp.DomainId!, spaceName, node.spaceApp.App!.AppType!) | ||
| await tryRemoteConnection(node, ctx, progress) | ||
| } catch (err: any) { | ||
| // Handle user declining instance type upgrade | ||
| if (err.code === InstanceTypeError) { | ||
| return | ||
| } | ||
| ) | ||
| } catch (err: any) { | ||
| // Ignore InstanceTypeError since it means the user decided not to use an instanceType with more memory | ||
| // just return without showing progress | ||
| if (err.code === InstanceTypeError) { | ||
| return | ||
| throw new ToolkitError(`Remote connection failed: ${err.message}`, { | ||
| cause: err, | ||
| code: err.code, | ||
| }) | ||
| } | ||
| throw new ToolkitError(`Remote connection failed: ${(err as Error).message}`, { | ||
| cause: err as Error, | ||
| code: err.code, | ||
| }) | ||
| } | ||
| } else if (node.getStatus() === 'Running') { | ||
| // For running spaces, show progress | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Handles connecting to a stopped space | ||
| * Starts the space and connects (remote access enabled automatically if needed) | ||
| */ | ||
| async function handleStoppedSpace( | ||
| node: SagemakerSpaceNode | SagemakerUnifiedStudioSpaceNode, | ||
| ctx: vscode.ExtensionContext, | ||
| spaceName: string, | ||
| sageMakerClient?: SagemakerClient | ||
| ) { | ||
| const client = sageMakerClient || new SagemakerClient(node.regionCode) | ||
|
|
||
bhavya2109sharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| try { | ||
| await client.startSpace(spaceName, node.spaceApp.DomainId!) | ||
| await tryRefreshNode(node) | ||
|
|
||
| return await vscode.window.withProgress( | ||
| { | ||
| location: vscode.ProgressLocation.Notification, | ||
| cancellable: false, | ||
| title: `Connecting to ${spaceName}`, | ||
| }, | ||
| async (progress) => { | ||
| progress.report({ message: 'Starting the space' }) | ||
| await client.waitForAppInService(node.spaceApp.DomainId!, spaceName, node.spaceApp.App!.AppType!) | ||
| await tryRemoteConnection(node, ctx, progress) | ||
| } | ||
| ) | ||
| } catch (err: any) { | ||
| // Handle user declining instance type upgrade | ||
| if (err.code === InstanceTypeError) { | ||
| return | ||
| } | ||
| throw new ToolkitError(`Remote connection failed: ${(err as Error).message}`, { | ||
| cause: err as Error, | ||
| code: err.code, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Handles connecting to a running space with enabled remote access | ||
| * Direct connection without any space modifications | ||
| */ | ||
| async function handleRunningSpaceWithEnabledAccess( | ||
| node: SagemakerSpaceNode | SagemakerUnifiedStudioSpaceNode, | ||
| ctx: vscode.ExtensionContext, | ||
| spaceName: string, | ||
| sageMakerClient?: SagemakerClient | ||
| ) { | ||
| return await vscode.window.withProgress( | ||
| { | ||
| location: vscode.ProgressLocation.Notification, | ||
| cancellable: false, | ||
| title: `Connecting to ${spaceName}`, | ||
| }, | ||
| async (progress) => { | ||
| await tryRemoteConnection(node, ctx, progress) | ||
| } | ||
| ) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import { getIcon, IconPath } from '../../shared/icons' | |
| import { generateSpaceStatus, updateIdleFile, startMonitoringTerminalActivity, ActivityCheckInterval } from './utils' | ||
| import { UserActivity } from '../../shared/extensionUtilities' | ||
| import { getLogger } from '../../shared/logger/logger' | ||
| import { SpaceStatus, RemoteAccess } from './constants' | ||
|
|
||
| export class SagemakerSpace { | ||
| public label: string = '' | ||
|
|
@@ -53,7 +54,7 @@ export class SagemakerSpace { | |
| } | ||
|
|
||
| public isPending(): boolean { | ||
| return this.getStatus() !== 'Running' && this.getStatus() !== 'Stopped' | ||
| return this.getStatus() !== SpaceStatus.RUNNING && this.getStatus() !== SpaceStatus.STOPPED | ||
| } | ||
|
|
||
| public getStatus(): string { | ||
|
|
@@ -148,8 +149,18 @@ export class SagemakerSpace { | |
| const domainId = this.spaceApp?.DomainId ?? '-' | ||
| const owner = this.spaceApp?.OwnershipSettingsSummary?.OwnerUserProfileName || '-' | ||
| const instanceType = this.spaceApp?.App?.ResourceSpec?.InstanceType ?? '-' | ||
| const remoteAccess = this.spaceApp?.SpaceSettingsSummary?.RemoteAccess | ||
|
|
||
| let baseTooltip = '' | ||
| if (this.isSMUSSpace) { | ||
| return `**Space:** ${spaceName} \n\n**Application:** ${appType} \n\n**Instance Type:** ${instanceType}` | ||
| baseTooltip = `**Space:** ${spaceName} \n\n**Application:** ${appType} \n\n**Instance Type:** ${instanceType}` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for updating this! |
||
| if (remoteAccess === RemoteAccess.ENABLED) { | ||
| baseTooltip += `\n\n**Remote Access:** Enabled` | ||
| } else if (remoteAccess === RemoteAccess.DISABLED) { | ||
| baseTooltip += `\n\n**Remote Access:** Disabled` | ||
| } | ||
|
|
||
| return baseTooltip | ||
| } | ||
| return `**Space:** ${spaceName} \n\n**Application:** ${appType} \n\n**Domain ID:** ${domainId} \n\n**User Profile:** ${owner}` | ||
| } | ||
|
|
@@ -166,20 +177,12 @@ export class SagemakerSpace { | |
|
|
||
| public getContext(): string { | ||
| const status = this.getStatus() | ||
| if (status === 'Running' && this.spaceApp.SpaceSettingsSummary?.RemoteAccess === 'ENABLED') { | ||
| return 'awsSagemakerSpaceRunningRemoteEnabledNode' | ||
| } else if (status === 'Running' && this.spaceApp.SpaceSettingsSummary?.RemoteAccess === 'DISABLED') { | ||
| return 'awsSagemakerSpaceRunningRemoteDisabledNode' | ||
| } else if (status === 'Running' && this.isSMUSSpace) { | ||
|
|
||
| // only distinguish between running and non-running states | ||
| if (status === SpaceStatus.RUNNING) { | ||
bhavya2109sharma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return 'awsSagemakerSpaceRunningNode' | ||
| } else if (status === 'Stopped' && this.spaceApp.SpaceSettingsSummary?.RemoteAccess === 'ENABLED') { | ||
| return 'awsSagemakerSpaceStoppedRemoteEnabledNode' | ||
| } else if ( | ||
| (status === 'Stopped' && !this.spaceApp.SpaceSettingsSummary?.RemoteAccess) || | ||
| this.spaceApp.SpaceSettingsSummary?.RemoteAccess === 'DISABLED' | ||
| ) { | ||
| return 'awsSagemakerSpaceStoppedRemoteDisabledNode' | ||
| } | ||
|
|
||
| return this.isSMUSSpace ? 'smusSpaceNode' : 'awsSagemakerSpaceNode' | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.