-
Notifications
You must be signed in to change notification settings - Fork 746
fix(sagemaker): fix sagemaker.parseCookies cmd not found #7670
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
Conversation
|
bc021f9 to
531270e
Compare
531270e to
2eb37d4
Compare
arkaprava08
left 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.
LGTM. Tested in remoteIDE environment
packages/core/src/auth/activation.ts
Outdated
| initializeCredentialsProviderManager() | ||
| } | ||
| } catch (e) { | ||
| getLogger().warn(`Failed to execute command "sagemaker.parseCookies": ${e}`) |
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.
We cannot assume all exceptions come from "sagemaker.parseCookies".
Please consider conditionally ignore the exception only for "sagemaker.parseCookies" issue. For other exceptions, we still need to throw the exception.
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.
updated the pr.
2eb37d4 to
f0a475d
Compare
|
Please add some UT and I can approve this PR. |
i will add ut in a follow up pr, and ask you to review. and for now i will merge the pr. |
|
UT is in another PR. |
## Problem This pr: #7670 didn't been covered by the unit test ## Solution Add unit test for the activation initialize method. --- - 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.
Problem
After Remote - SSH to a sagemaker space via VS Code, the amazon Q plugin cannot be loaded for the following error.
Solution
After discussed with the Sagemaker team folks @sunp and @arkaprav, we decide that if cmd
sagemaker.parseCookiesnot found, we just swallow the error and add a log for it.Test
a local build is created for testing.

feature/xbranches will not be squash-merged at release time.