-
Notifications
You must be signed in to change notification settings - Fork 748
fix(amazonq): fix for amazon q app initialization failure on sagemaker #7548
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
Changes from all commits
3a4ddc9
ebadabc
a7e37a3
e6c085c
1656f58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "Bug Fix", | ||
| "description": "Improved Amazon Linux 2 support with better SageMaker environment detection" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -526,7 +526,7 @@ | |
| "@aws-sdk/s3-request-presigner": "<3.731.0", | ||
| "@aws-sdk/smithy-client": "<3.731.0", | ||
| "@aws-sdk/util-arn-parser": "<3.731.0", | ||
| "@aws/mynah-ui": "^4.35.4", | ||
| "@aws/mynah-ui": "^4.35.6", | ||
|
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. mynah update should not be in this focused change
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. sorry..previous commit sneaked in...let me correct and raise a new one |
||
| "@gerhobbelt/gitignore-parser": "^0.2.0-9", | ||
| "@iarna/toml": "^2.2.5", | ||
| "@smithy/fetch-http-handler": "^5.0.1", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,6 +176,18 @@ export function isCloud9(flavor: 'classic' | 'codecatalyst' | 'any' = 'any'): bo | |
| * @returns true if the current system is SageMaker(SMAI or SMUS) | ||
| */ | ||
| export function isSageMaker(appName: 'SMAI' | 'SMUS' = 'SMAI'): boolean { | ||
| // Check for SageMaker-specific environment variables first | ||
|
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. FYI, the check for Unified Studio i.e |
||
| if ( | ||
| process.env.SAGEMAKER_APP_TYPE !== undefined || | ||
| process.env.SERVICE_NAME === sageMakerUnifiedStudio || | ||
| process.env.SAGEMAKER_INTERNAL_IMAGE_URI !== undefined || | ||
| process.env.STUDIO_LOGGING_DIR?.includes('/var/log/studio') === true | ||
| ) { | ||
| getLogger().debug('SageMaker environment detected via environment variables') | ||
| return true | ||
| } | ||
|
|
||
| // Fall back to app name checks | ||
| switch (appName) { | ||
| case 'SMAI': | ||
| return vscode.env.appName === sageMakerAppname | ||
|
|
||
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.
it appears that the
isAmazonLinux2check is incorrectly flagging ubuntu containers under AL2 hosts. i do not think we need to special case sagemaker for nowThere 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.
what is the exact setup for SageMaker AL2 instances?