Skip to content

Commit 3b2f8b3

Browse files
authored
Merge pull request aws#8292 from bhavya2109sharma/fix/riv-bugs-clean
fix(sagemaker): better UX exp for user facing messages.
2 parents e369ff3 + 419a727 commit 3b2f8b3

File tree

6 files changed

+17
-14
lines changed

6 files changed

+17
-14
lines changed

packages/core/src/awsService/sagemaker/commands.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ async function handleRunningSpaceWithDisabledAccess(
301301

302302
const confirmed = await showConfirmationMessage({
303303
prompt,
304-
confirm: 'Restart and Connect',
304+
confirm: 'Restart Space and Connect',
305305
cancel: 'Cancel',
306306
type: 'warning',
307307
})

packages/core/src/awsService/sagemaker/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export const InstanceTypeInsufficientMemoryMessage = (
3636
chosenInstanceType: string,
3737
recommendedInstanceType: string
3838
) => {
39-
return `Unable to create app for [${spaceName}] because instanceType [${chosenInstanceType}] is not supported for remote access enabled spaces. Use instanceType with at least 8 GiB memory. Would you like to start your space with instanceType [${recommendedInstanceType}]?`
39+
return `[${chosenInstanceType}] does not support remote access. Use an instanceType with at least 8 GiB memory. Would you like to start your space with instanceType [${recommendedInstanceType}]?`
4040
}
4141

4242
export const InstanceTypeNotSelectedMessage = (spaceName: string) => {

packages/core/src/sagemakerunifiedstudio/explorer/nodes/sageMakerUnifiedStudioProjectNode.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ export class SageMakerUnifiedStudioProjectNode implements TreeNode {
132132
if (this.isFirstTimeSelection && !this.hasShownFirstTimeMessage) {
133133
this.hasShownFirstTimeMessage = true
134134
void vscode.window.showInformationMessage(
135-
'Find your space in the Explorer panel under SageMaker Unified Studio. Hover over any space and click the connection icon to connect remotely.'
135+
'Find your space in the Explorer panel under SageMaker Unified Studio. Hover over a space and click the connection icon to connect remotely.'
136136
)
137137
}
138138
this.sagemakerClient = await this.initializeSagemakerClient(spaceAwsAccountRegion)

packages/core/src/shared/clients/sagemaker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ export class SagemakerClient extends ClientWrapper<SageMakerClient> {
180180
instanceType,
181181
InstanceTypeInsufficientMemory[instanceType]
182182
),
183-
confirm: 'Restart and Connect',
183+
confirm: 'Restart Space and Connect',
184184
cancel: 'Cancel',
185185
type: 'warning',
186186
})

packages/core/src/test/awsService/sagemaker/commands.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ describe('SageMaker Commands', () => {
124124
// Setup test window to handle confirmation dialog
125125
getTestWindow().onDidShowMessage((message) => {
126126
if (message.message.includes(RemoteAccessRequiredMessage)) {
127-
message.selectItem('Restart and Connect')
127+
message.selectItem('Restart Space and Connect')
128128
}
129129
})
130130

@@ -182,7 +182,7 @@ describe('SageMaker Commands', () => {
182182
InstanceTypeInsufficientMemoryMessage('test-space', 'ml.t3.medium', 'ml.t3.large')
183183
)
184184
) {
185-
message.selectItem('Restart and Connect')
185+
message.selectItem('Restart Space and Connect')
186186
}
187187
})
188188

@@ -236,8 +236,8 @@ describe('SageMaker Commands', () => {
236236

237237
// Setup test window to confirm
238238
getTestWindow().onDidShowMessage((message) => {
239-
if (message.items.some((item) => item.title === 'Restart and Connect')) {
240-
message.selectItem('Restart and Connect')
239+
if (message.items.some((item) => item.title === 'Restart Space and Connect')) {
240+
message.selectItem('Restart Space and Connect')
241241
}
242242
})
243243

@@ -337,7 +337,7 @@ describe('SageMaker Commands', () => {
337337

338338
// Verify no confirmation dialog shown for stopped space
339339
const confirmMessages = getTestWindow().shownMessages.filter((m) =>
340-
m.message.includes('Restart and Connect')
340+
m.message.includes('Restart Space and Connect')
341341
)
342342
assert.strictEqual(confirmMessages.length, 0, 'Should not show confirmation for stopped space')
343343

@@ -388,7 +388,7 @@ describe('SageMaker Commands', () => {
388388
assert(mockTryRefreshNode.calledOnce)
389389
// Verify no confirmation needed
390390
const confirmMessages = getTestWindow().shownMessages.filter((m) =>
391-
m.message.includes('Restart and Connect')
391+
m.message.includes('Restart Space and Connect')
392392
)
393393
assert.strictEqual(confirmMessages.length, 0)
394394
// Verify no space operations performed

packages/core/src/test/shared/clients/sagemakerClient.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { DescribeDomainResponse } from '@amzn/sagemaker-client'
1111
import { intoCollection } from '../../../shared/utilities/collectionUtils'
1212
import { ToolkitError } from '../../../shared/errors'
1313
import { getTestWindow } from '../vscode/window'
14+
import { InstanceTypeInsufficientMemoryMessage } from '../../../awsService/sagemaker/constants'
1415

1516
describe('SagemakerClient.fetchSpaceAppsAndDomains', function () {
1617
const region = 'test-region'
@@ -398,9 +399,10 @@ describe('SagemakerClient.startSpace', function () {
398399

399400
const promise = client.startSpace('my-space', 'my-domain')
400401

401-
// Wait for the error message to appear and select "Restart and Connect"
402-
await getTestWindow().waitForMessage(/not supported for remote access/)
403-
getTestWindow().getFirstMessage().selectItem('Restart and Connect')
402+
// Wait for the error message to appear and select "Restart Space and Connect"
403+
const expectedMessage = InstanceTypeInsufficientMemoryMessage('my-space', 'ml.t3.medium', 'ml.t3.large')
404+
await getTestWindow().waitForMessage(new RegExp(expectedMessage.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')))
405+
getTestWindow().getFirstMessage().selectItem('Restart Space and Connect')
404406

405407
await promise
406408
sinon.assert.calledOnce(updateSpaceStub)
@@ -424,7 +426,8 @@ describe('SagemakerClient.startSpace', function () {
424426
const promise = client.startSpace('my-space', 'my-domain')
425427

426428
// Wait for the error message to appear and select "Cancel"
427-
await getTestWindow().waitForMessage(/not supported for remote access/)
429+
const expectedMessage = InstanceTypeInsufficientMemoryMessage('my-space', 'ml.t3.medium', 'ml.t3.large')
430+
await getTestWindow().waitForMessage(new RegExp(expectedMessage.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')))
428431
getTestWindow().getFirstMessage().selectItem('Cancel')
429432

430433
await assert.rejects(promise, (err: ToolkitError) => err.message === 'InstanceType has insufficient memory.')

0 commit comments

Comments
 (0)