Skip to content

Commit 87c4eb8

Browse files
aws-asoliduNewton DerNewtonDerlaileni-aws
authored
fix(sagemaker): Add code-editor subdomain to refreshUrl (#7826)
## Problem -All space reconnection redirect links were hardcoded to route users to the JupyterLab details page, due to a bug in the Studio web application. ## Solution - With the bug now fixed in SageMaker Studio, we can support redirecting to the Code Editor details page via refreshUrl. - For backward compatibility, the default remains set to JupyterLab. Once the fix is verified and rolled out across all regions, we can remove the default behavior and instead explicitly check for the appType (e.g., JupyterLab) before setting the redirect. --- - 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: Newton Der <[email protected]> Co-authored-by: Newton Der <[email protected]> Co-authored-by: Laxman Reddy <[email protected]>
1 parent b488df4 commit 87c4eb8

File tree

5 files changed

+39
-12
lines changed

5 files changed

+39
-12
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ export async function deeplinkConnect(
8686
session: string,
8787
wsUrl: string,
8888
token: string,
89-
domain: string
89+
domain: string,
90+
appType?: string
9091
) {
9192
getLogger().debug(
9293
`sm:deeplinkConnect: connectionIdentifier: ${connectionIdentifier} session: ${session} wsUrl: ${wsUrl} token: ${token}`
@@ -107,7 +108,8 @@ export async function deeplinkConnect(
107108
session,
108109
wsUrl,
109110
token,
110-
domain
111+
domain,
112+
appType
111113
)
112114

113115
await startVscodeRemote(

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,16 @@ export async function persistSSMConnection(
9696
domain: string,
9797
session?: string,
9898
wsUrl?: string,
99-
token?: string
99+
token?: string,
100+
appType?: string
100101
): Promise<void> {
101102
const { region } = parseArn(spaceArn)
102103
const endpoint = DevSettings.instance.get('endpoints', {})['sagemaker'] ?? ''
103104

104-
// TODO: Hardcoded to 'jupyterlab' due to a bug in Studio that only supports refreshing
105-
// the token for both CodeEditor and JupyterLab Apps in the jupyterlab subdomain.
106-
// This will be fixed shortly after NYSummit launch to support refresh URL in CodeEditor subdomain.
107-
const appSubDomain = 'jupyterlab'
105+
let appSubDomain = 'jupyterlab'
106+
if (appType && appType.toLowerCase() === 'codeeditor') {
107+
appSubDomain = 'code-editor'
108+
}
108109

109110
let envSubdomain: string
110111

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ export async function prepareDevEnvConnection(
5656
session?: string,
5757
wsUrl?: string,
5858
token?: string,
59-
domain?: string
59+
domain?: string,
60+
appType?: string
6061
) {
6162
const remoteLogger = configureRemoteConnectionLogger()
6263
const { ssm, vsc, ssh } = (await ensureDependencies()).unwrap()
@@ -82,7 +83,7 @@ export async function prepareDevEnvConnection(
8283
await persistSmusProjectCreds(spaceArn, node as SagemakerUnifiedStudioSpaceNode)
8384
}
8485
} else if (connectionType === 'sm_dl') {
85-
await persistSSMConnection(spaceArn, domain ?? '', session, wsUrl, token)
86+
await persistSSMConnection(spaceArn, domain ?? '', session, wsUrl, token, appType)
8687
}
8788

8889
await startLocalServer(ctx)

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ export function register(ctx: ExtContext) {
1818
params.session,
1919
`${params.ws_url}&cell-number=${params['cell-number']}`,
2020
params.token,
21-
params.domain
21+
params.domain,
22+
params.app_type
2223
)
2324
})
2425
}
@@ -27,7 +28,7 @@ export function register(ctx: ExtContext) {
2728
}
2829

2930
export function parseConnectParams(query: SearchParams) {
30-
const params = query.getFromKeysOrThrow(
31+
const requiredParams = query.getFromKeysOrThrow(
3132
'connection_identifier',
3233
'domain',
3334
'user_profile',
@@ -36,5 +37,7 @@ export function parseConnectParams(query: SearchParams) {
3637
'cell-number',
3738
'token'
3839
)
39-
return params
40+
const optionalParams = query.getFromKeys('app_type')
41+
42+
return { ...requiredParams, ...optionalParams }
4043
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ describe('SageMaker URI handler', function () {
4444
ws_url: 'wss://example.com',
4545
'cell-number': '4',
4646
token: 'my-token',
47+
app_type: 'jupyterlab',
4748
}
4849

4950
const uri = createConnectUri(params)
@@ -55,5 +56,24 @@ describe('SageMaker URI handler', function () {
5556
assert.deepStrictEqual(deeplinkConnectStub.firstCall.args[3], 'wss://example.com&cell-number=4')
5657
assert.deepStrictEqual(deeplinkConnectStub.firstCall.args[4], 'my-token')
5758
assert.deepStrictEqual(deeplinkConnectStub.firstCall.args[5], 'my-domain')
59+
assert.deepStrictEqual(deeplinkConnectStub.firstCall.args[6], 'jupyterlab')
60+
})
61+
62+
it('calls deeplinkConnect with undefined app_type when not provided', async function () {
63+
const params = {
64+
connection_identifier: 'abc123',
65+
domain: 'my-domain',
66+
user_profile: 'me',
67+
session: 'sess-xyz',
68+
ws_url: 'wss://example.com',
69+
'cell-number': '4',
70+
token: 'my-token',
71+
}
72+
73+
const uri = createConnectUri(params)
74+
await handler.handleUri(uri)
75+
76+
assert.ok(deeplinkConnectStub.calledOnce)
77+
assert.deepStrictEqual(deeplinkConnectStub.firstCall.args[6], undefined)
5878
})
5979
})

0 commit comments

Comments
 (0)