Skip to content

Commit c0d1e99

Browse files
fix(smus): Space metadata is stale after the updateSpace call (aws#2213)
## Problem Consistently reproducing a similar issue when attempting to start/connect to a Stopped Space when remote access is initially disabled. The Space is updated, started, and even connected to, but the Space does not show the Connect button anymore, until the Project is refreshed. This suggests the Space metadata is stale after the UpdateSpace call. ## Solution Tried to simply replace the spaceApp variable with the updated one got from describeSpace API call, but find conflicts on variable definitions. so update the variable names and update only remoteAccess variable to minimize impact. ListSpaces(original) https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/sagemaker/command/ListSpacesCommand/ <img width="2556" height="1308" alt="Screenshot 2025-09-01 at 4 11 09 PM" src="https://github.com/user-attachments/assets/64b4439e-4495-4822-9a56-c61470e269c6" /> DescribeSpace(updated) https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-sagemaker/Interface/DescribeSpaceCommandOutput/ <img width="2523" height="1299" alt="Screenshot 2025-09-01 at 5 02 24 PM" src="https://github.com/user-attachments/assets/37860497-8116-4d16-bc08-2562d0ea25bd" /> - pass SpaceSettingsSummary -> SpaceSettings in spaceApp type parameter - update remote access ## Test manually debug and unit test --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. Co-authored-by: Laxman Reddy <[email protected]>
1 parent 5c8fc36 commit c0d1e99

File tree

2 files changed

+146
-1
lines changed

2 files changed

+146
-1
lines changed

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ export class SagemakerSpace {
3333

3434
public updateSpace(spaceApp: SagemakerSpaceApp) {
3535
this.setSpaceStatus(spaceApp.Status ?? '', spaceApp.App?.Status ?? '')
36+
// Only update RemoteAccess property to minimize impact due to minor structural differences between variables
37+
if (this.spaceApp.SpaceSettingsSummary && spaceApp.SpaceSettingsSummary?.RemoteAccess) {
38+
this.spaceApp.SpaceSettingsSummary.RemoteAccess = spaceApp.SpaceSettingsSummary.RemoteAccess
39+
}
3640
this.label = this.buildLabel()
3741
this.description = this.isSMUSSpace ? undefined : this.buildDescription()
3842
this.tooltip = new vscode.MarkdownString(this.buildTooltip())
@@ -109,8 +113,20 @@ export class SagemakerSpace {
109113
SpaceName: this.spaceApp.SpaceName,
110114
})
111115

116+
// AWS DescribeSpace API returns full details with property names like 'SpaceSettings'
117+
// but our internal SagemakerSpaceApp type expects 'SpaceSettingsSummary' (from ListSpaces API)
118+
// We destructure and rename properties to maintain type compatibility
119+
const {
120+
SpaceSettings: spaceSettingsSummary,
121+
OwnershipSettings: ownershipSettingsSummary,
122+
SpaceSharingSettings: spaceSharingSettingsSummary,
123+
...spaceDetails
124+
} = space
112125
this.updateSpace({
113-
...space,
126+
SpaceSettingsSummary: spaceSettingsSummary,
127+
OwnershipSettingsSummary: ownershipSettingsSummary,
128+
SpaceSharingSettingsSummary: spaceSharingSettingsSummary,
129+
...spaceDetails,
114130
App: app,
115131
DomainSpaceKey: this.spaceApp.DomainSpaceKey,
116132
})
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import assert from 'assert'
7+
import { SagemakerSpace } from '../../../awsService/sagemaker/sagemakerSpace'
8+
import { SagemakerClient, SagemakerSpaceApp } from '../../../shared/clients/sagemaker'
9+
import sinon from 'sinon'
10+
11+
describe('SagemakerSpace', function () {
12+
let mockClient: sinon.SinonStubbedInstance<SagemakerClient>
13+
let mockSpaceApp: SagemakerSpaceApp
14+
15+
beforeEach(function () {
16+
mockClient = sinon.createStubInstance(SagemakerClient)
17+
mockSpaceApp = {
18+
SpaceName: 'test-space',
19+
Status: 'InService',
20+
DomainId: 'test-domain',
21+
DomainSpaceKey: 'test-key',
22+
SpaceSettingsSummary: {
23+
AppType: 'JupyterLab',
24+
RemoteAccess: 'ENABLED',
25+
},
26+
}
27+
})
28+
29+
afterEach(function () {
30+
sinon.restore()
31+
})
32+
33+
describe('updateSpaceAppStatus', function () {
34+
it('should correctly map DescribeSpace API response to SagemakerSpaceApp type', async function () {
35+
// Mock DescribeSpace response (uses full property names)
36+
const mockDescribeSpaceResponse = {
37+
SpaceName: 'updated-space',
38+
Status: 'InService',
39+
DomainId: 'test-domain',
40+
SpaceSettings: {
41+
// Note: 'SpaceSettings' not 'SpaceSettingsSummary'
42+
AppType: 'CodeEditor',
43+
RemoteAccess: 'DISABLED',
44+
},
45+
OwnershipSettings: {
46+
OwnerUserProfileName: 'test-user',
47+
},
48+
SpaceSharingSettings: {
49+
SharingType: 'Private',
50+
},
51+
$metadata: { requestId: 'test-request-id' },
52+
}
53+
54+
// Mock DescribeApp response
55+
const mockDescribeAppResponse = {
56+
AppName: 'test-app',
57+
Status: 'InService',
58+
ResourceSpec: {
59+
InstanceType: 'ml.t3.medium',
60+
},
61+
$metadata: { requestId: 'test-request-id' },
62+
}
63+
64+
mockClient.describeSpace.resolves(mockDescribeSpaceResponse)
65+
mockClient.describeApp.resolves(mockDescribeAppResponse)
66+
67+
const space = new SagemakerSpace(mockClient as any, 'us-east-1', mockSpaceApp)
68+
const updateSpaceSpy = sinon.spy(space, 'updateSpace')
69+
70+
await space.updateSpaceAppStatus()
71+
72+
// Verify updateSpace was called with correctly mapped properties
73+
assert.ok(updateSpaceSpy.calledOnce)
74+
const updateSpaceArgs = updateSpaceSpy.getCall(0).args[0]
75+
76+
// Verify property name mapping from DescribeSpace to SagemakerSpaceApp
77+
assert.strictEqual(updateSpaceArgs.SpaceSettingsSummary?.AppType, 'CodeEditor')
78+
assert.strictEqual(updateSpaceArgs.SpaceSettingsSummary?.RemoteAccess, 'DISABLED')
79+
assert.strictEqual(updateSpaceArgs.OwnershipSettingsSummary?.OwnerUserProfileName, 'test-user')
80+
assert.strictEqual(updateSpaceArgs.SpaceSharingSettingsSummary?.SharingType, 'Private')
81+
82+
// Verify other properties are preserved
83+
assert.strictEqual(updateSpaceArgs.SpaceName, 'updated-space')
84+
assert.strictEqual(updateSpaceArgs.Status, 'InService')
85+
assert.strictEqual(updateSpaceArgs.DomainId, 'test-domain')
86+
assert.strictEqual(updateSpaceArgs.App, mockDescribeAppResponse)
87+
assert.strictEqual(updateSpaceArgs.DomainSpaceKey, 'test-key')
88+
89+
// Verify original API property names are not present
90+
assert.ok(!('SpaceSettings' in updateSpaceArgs))
91+
assert.ok(!('OwnershipSettings' in updateSpaceArgs))
92+
assert.ok(!('SpaceSharingSettings' in updateSpaceArgs))
93+
})
94+
95+
it('should handle missing optional properties gracefully', async function () {
96+
// Mock minimal DescribeSpace response
97+
const mockDescribeSpaceResponse = {
98+
SpaceName: 'minimal-space',
99+
Status: 'InService',
100+
DomainId: 'test-domain',
101+
$metadata: { requestId: 'test-request-id' },
102+
// No SpaceSettings, OwnershipSettings, or SpaceSharingSettings
103+
}
104+
105+
const mockDescribeAppResponse = {
106+
AppName: 'test-app',
107+
Status: 'InService',
108+
$metadata: { requestId: 'test-request-id' },
109+
}
110+
111+
mockClient.describeSpace.resolves(mockDescribeSpaceResponse)
112+
mockClient.describeApp.resolves(mockDescribeAppResponse)
113+
114+
const space = new SagemakerSpace(mockClient as any, 'us-east-1', mockSpaceApp)
115+
const updateSpaceSpy = sinon.spy(space, 'updateSpace')
116+
117+
await space.updateSpaceAppStatus()
118+
119+
// Should not throw and should handle undefined properties
120+
assert.ok(updateSpaceSpy.calledOnce)
121+
const updateSpaceArgs = updateSpaceSpy.getCall(0).args[0]
122+
123+
assert.strictEqual(updateSpaceArgs.SpaceName, 'minimal-space')
124+
assert.strictEqual(updateSpaceArgs.SpaceSettingsSummary, undefined)
125+
assert.strictEqual(updateSpaceArgs.OwnershipSettingsSummary, undefined)
126+
assert.strictEqual(updateSpaceArgs.SpaceSharingSettingsSummary, undefined)
127+
})
128+
})
129+
})

0 commit comments

Comments
 (0)