-
Notifications
You must be signed in to change notification settings - Fork 83
feat: Adding ATX-FES apis support for transformation #2521
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
* feat: add atx fes integration for transform profiles * feat: implement Transform profile discovery via ATX FES with cache clearing * fix: remove unsupported eu-central-1 region from ATX FES endpoints * feat: add separate flow for RTS and ATX listavailableprofile api * fix: remove profile handling from atxnettransformserver * feat: separating qdev and aws transform * fix: fixing unit tests * fix: adding tests * fix: updating as per langugae server runtime updates * feat: add starttranform and workspace * feat: added getTransformInfo and its support methods * fix: with new runtimes * feat: add stopjob support * merged stopjob and added upload plan * chore: force use of new runtimes * fix: completed getting plan, worklogs, and final artifact * chore: deleting unused RPC messages * feat: added list worklogs before planning * fix: remove unused methods --------- Co-authored-by: Pranav Firake <[email protected]> Co-authored-by: pranav firake <[email protected]> Co-authored-by: Jordan Miao <[email protected]>
90f1003 to
767206f
Compare
…servers into karsraja-atx-final
…and prevent us-east-1 defaults
…servers into karsraja-atx-final
.../aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts.backup
Outdated
Show resolved
Hide resolved
…ase server is initialized
| return { | ||
| version, | ||
| servers: [AmazonQServiceServer, ...servers], | ||
| servers: [AmazonQServiceServer as any, ...servers], |
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.
why change this to as any
| export const createIAMRuntimeProps = createRuntimePropsFactory(AmazonQServiceServerIAM as any) | ||
| export const createTokenRuntimeProps = createRuntimePropsFactory(AmazonQServiceServerToken as any) |
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.
why are there all these casts to any?
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.
reverted this change.
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 still have these casts in 4 files
| AtxTokenServiceManager.initInstance({ | ||
| credentialsProvider, | ||
| lsp, | ||
| workspace, | ||
| logging, | ||
| runtime, | ||
| sdkInitializator, | ||
| }) |
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.
reiterating my previous request that all ATX-related changes to initialized by isolated into its own server factory
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.
yes Richard, we will refactor this as fast follow up iterm.
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.
Could yo update the comments with Todo? We don't want this part to be forgotten in the codebase
| * PENDING_Q_PROFILE_UPDATE (only for identityCenter connection) waiting for Developer Profile to complete | ||
| * INITIALIZED - Service is initialized | ||
| */ | ||
|
|
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.
?
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.
can we keep this comment
| this.cancelActiveProfileChangeToken() | ||
| if (type === ('bearer' as CredentialsType)) { | ||
| // Only clear Q state when Q credentials are deleted | ||
| const hasQCredentials = this.features.credentialsProvider.hasCredentials('bearer' as CredentialsType) |
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.
i thought we established that both atx and q are of bearer tokens
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.
reverted this change.
server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts
Outdated
Show resolved
Hide resolved
| public getAtxCodewhispererService(): CodeWhispererServiceToken { | ||
| throw new Error( | ||
| 'ATX functionality has been moved to AtxTokenServiceManager. Use AtxTokenServiceManager.getInstance().getAtxCodewhispererService() instead.' | ||
| ) | ||
| } |
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.
given that this was never in the q token manager service, why is this here?
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.
reverted this change.
server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts
Outdated
Show resolved
Hide resolved
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.
since this is not q, why is this in the q folder?
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 folder
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.
awsTransform or netTransform? also should move AtxTokenServiceManager and atxTransformProfiles?
| }, | ||
| "dependencies": { | ||
| "@aws/language-server-runtimes": "0.3.6", | ||
| "@aws/language-server-runtimes": "0.3.8", |
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.
for all instances, use the caret notation. we rely on package-lock.json for the actual resolved version.
| "@aws/language-server-runtimes": "0.3.8", | |
| "@aws/language-server-runtimes": "^0.3.8", |
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 this
| it('returns true when bearer-alternate credentials exist', () => { | ||
| features.credentialsProvider.hasCredentials.withArgs('bearer-alternate' as any).returns(true) |
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.
why do we still have all these references to 'bearer-alternate'? i thought this was now called 'atx-bearer'
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.
reverted this change
| public static initInstance(features: QServiceManagerFeatures): AtxTokenServiceManager { | ||
| if (!AtxTokenServiceManager.instance) { | ||
| AtxTokenServiceManager.instance = new AtxTokenServiceManager(features) | ||
| return AtxTokenServiceManager.instance | ||
| } | ||
| // throw new Error('ATX Token Service Manager already initialized') | ||
| return AtxTokenServiceManager.instance | ||
| } | ||
|
|
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 is a little bit concerned
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2521 +/- ##
==========================================
- Coverage 62.37% 60.13% -2.25%
==========================================
Files 270 276 +6
Lines 61794 65004 +3210
Branches 4063 4111 +48
==========================================
+ Hits 38546 39088 +542
- Misses 23165 25833 +2668
Partials 83 83
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
awsTransform or netTransform? also should move AtxTokenServiceManager and atxTransformProfiles?
| export const createIAMRuntimeProps = createRuntimePropsFactory(AmazonQServiceServerIAM as any) | ||
| export const createTokenRuntimeProps = createRuntimePropsFactory(AmazonQServiceServerToken as any) |
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 still have these casts in 4 files
| "dependencies": { | ||
| "@aws/hello-world-lsp": "^0.0.1", | ||
| "@aws/language-server-runtimes": "0.3.6" | ||
| "@aws/language-server-runtimes": "0.3.8" |
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.
missed this one
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 both
| * PENDING_Q_PROFILE_UPDATE (only for identityCenter connection) waiting for Developer Profile to complete | ||
| * INITIALIZED - Service is initialized | ||
| */ | ||
|
|
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.
can we keep this comment
server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts
Outdated
Show resolved
Hide resolved
fe41f3d to
6353ef2
Compare
| private pushedCredentials: IamCredentials | undefined | ||
| private pushedToken: BearerToken | undefined | ||
|
|
||
| private pushedAtxCredentials: IamCredentials | undefined |
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.
| private pushedAtxCredentials: IamCredentials | undefined |
| if (qCreds && 'token' in qCreds && qCreds.token) { | ||
| this.logging.log(`Q token updated: ${qCreds.token.substring(0, 20)}...`) | ||
| } |
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.
| if (qCreds && 'token' in qCreds && qCreds.token) { | |
| this.logging.log(`Q token updated: ${qCreds.token.substring(0, 20)}...`) | |
| } |
| const atxCredentialsProvider = runtime.getAtxCredentialsProvider() | ||
| this.log(`ATX credentials provider: ${!!atxCredentialsProvider}`) | ||
| const credentials = atxCredentialsProvider?.getCredentials('bearer') | ||
| this.log(`ATX credentials: ${!!credentials}`) |
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.log(`ATX credentials: ${!!credentials}`) |
feat: add atx fes integration for transform profiles
feat: implement Transform profile discovery via ATX FES with cache clearing
fix: remove unsupported eu-central-1 region from ATX FES endpoints
feat: add separate flow for RTS and ATX listavailableprofile api
fix: remove profile handling from atxnettransformserver
feat: separating qdev and aws transform
fix: fixing unit tests
fix: adding tests
fix: updating as per langugae server runtime updates
feat: add starttranform and workspace
feat: added getTransformInfo and its support methods
fix: with new runtimes
feat: add stopjob support
merged stopjob and added upload plan
chore: force use of new runtimes
fix: completed getting plan, worklogs, and final artifact
chore: deleting unused RPC messages
feat: added list worklogs before planning
fix: remove unused methods
Problem
Solution
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.