fix(amazonq): acquire Toolkit api on-demand #4874
Conversation
justinmk3
force-pushed
the
gettoolkitapi
branch
from
e9a81f4
to
60b1f40
Compare
| label: conn.label, | ||
| } as AwsConnection) | ||
| } | ||
| const toolkitApi = getToolkitApi() |
There was a problem hiding this comment.
Get the API on-demand. Simpler, fewer edge cases, reliable, easier to reason about.
| return undefined | ||
| } | ||
| await activateExtension(VSCODE_EXTENSION_ID.awstoolkit) |
There was a problem hiding this comment.
Could add this back, but it's kind of pointless because Toolkit activates on onStartupFinished.
Problem:
The logic to get the Toolkit API is spread around and involves:
- a "aws.amazonq.refreshConnectionCallback" command (silent failures,
not strongly typed, etc)
- numerous attempts to "eagerly" activate Toolkit
- Toolkit's own startup passes the toolkit API as an object via the "aws.amazonq.refreshConnectionCallback" command
- this might not work if the object doesn't serialize correctly, e.g.
for web workers or when the vscode "extensions.experimental.affinity" behavior is enabled.
Solution:
Instead of trying to eagerly activate Toolkit, just get the Toolkit API
on-demand. This avoids complexity and is more reliable.
justinmk3
force-pushed
the
gettoolkitapi
branch
from
60b1f40
to
1f77a2a
Compare
|
Can you also update how toolkit api is acquired in |
| @@ -430,27 +459,6 @@ const registerToolkitApiCallbackOnce = once(async () => { | |||
| } | |||
| ) | |||
| } | |||
| }) | |||
| export const registerToolkitApiCallback = Commands.declare( | |||
| { id: 'aws.amazonq.refreshConnectionCallback' }, | |||
There was a problem hiding this comment.
When this command is removed, this change has to go through both toolkit and Q. Assume it is released in 1.0.1 Q and 3.0.1 Toolkit, then there can be command not found issue when using 1.0.1 Q with 3.0.0 toolkit or using 1.0.0 Q with 3.0.1 toolkit. We do wrap it under tryExecute so user should not see a command not found popup. This is not a blocker issue since user can upgrade both (but both needs to go out at same time)
(This can wait until after release)
Problem:
The logic to get the Toolkit API is spread around and involves:
Solution:
Instead of trying to eagerly activate Toolkit, just get the Toolkit API on-demand. This avoids complexity and is more reliable.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.