-
Notifications
You must be signed in to change notification settings - Fork 737
feat(sagemaker): free tier Q Chat with auto-login for iam users and login option for pro tier users #5858
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
feat(sagemaker): free tier Q Chat with auto-login for iam users and login option for pro tier users #5858
Changes from all commits
684853d
ea1e8d2
4fdbe1d
15b819c
8c745f4
6f33b6c
f6968b6
88831d3
04d4cf8
4ff2964
4da8f68
4eb15e2
abff521
3028316
e877994
5f33310
6af05e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "Bug Fix", | ||
| "description": "Use Sagemaker environment IAM Credentials for Code Completion when they're available" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "Bug Fix", | ||
| "description": "Inline: Code completion not working for Sagemaker Pro Tier users." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "Bug Fix", | ||
| "description": "Disable /transform and /dev commands for sagemaker users as they're not supported" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "Feature", | ||
| "description": "Enable Free Tier Chat for IAM users" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,16 +3,28 @@ | |
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import * as vscode from 'vscode' | ||
| import { Auth } from './auth' | ||
| import { LoginManager } from './deprecated/loginManager' | ||
| import { fromString } from './providers/credentials' | ||
| import { getLogger } from '../shared/logger' | ||
| import { ExtensionUse } from './utils' | ||
| import { isCloud9 } from '../shared/extensionUtilities' | ||
| import { ExtensionUse, initializeCredentialsProviderManager } from './utils' | ||
| import { isAmazonQ, isCloud9, isSageMaker } from '../shared/extensionUtilities' | ||
| import { isInDevEnv } from '../shared/vscode/env' | ||
| import { isWeb } from '../shared/extensionGlobals' | ||
|
|
||
| interface SagemakerCookie { | ||
| authMode?: 'Sso' | 'Iam' | ||
| } | ||
|
|
||
| export async function initialize(loginManager: LoginManager): Promise<void> { | ||
| if (isAmazonQ() && isSageMaker()) { | ||
| // The command `sagemaker.parseCookies` is registered in VS Code Sagemaker environment. | ||
| const result = (await vscode.commands.executeCommand('sagemaker.parseCookies')) as SagemakerCookie | ||
| if (result.authMode !== 'Sso') { | ||
| initializeCredentialsProviderManager() | ||
|
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. how does this refactor affect toolkit ? In sagemaker studio sso mode, toolkit will still work in IAM mode.
Contributor
Author
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. The higher level condition is scoped to amazonq, so it shouldn't impact aws toolkit |
||
| } | ||
| } | ||
| Auth.instance.onDidChangeActiveConnection(async (conn) => { | ||
| // This logic needs to be moved to `Auth.useConnection` to correctly record `passive` | ||
| if (conn?.type === 'iam' && conn.state === 'valid') { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this command come from? Is it already registered in VSC by sagemaker? If so add a comment to mention this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, this command is registered in Sagemaker code editor. I'll add a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets make a new sagemaker folder in
src/shared/sagemakerfor all custom sagemaker code so that it is contained within a single place.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a big refactor that I'd rather defer to a later followup to reduce risk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a long-term approach. The Toolkit/Q extensions already have an api to accept connections. If needed, they can be enhanced to accept tokens/credentials, so the caller (SM in this case) can inject credentials into the extension.