fix supplemental context being built in EDT #4765
Conversation
...y/src/software/aws/toolkits/jetbrains/services/codewhisperer/service/CodeWhispererService.kt
Outdated
Show resolved
Hide resolved
...tware/aws/toolkits/jetbrains/services/codewhisperer/util/CodeWhispererFileContextProvider.kt
Outdated
Show resolved
Hide resolved
...tware/aws/toolkits/jetbrains/services/codewhisperer/util/CodeWhispererFileContextProvider.kt
Outdated
Show resolved
Hide resolved
| val contentLength: Int | ||
| get() = contents.fold(0) { acc, chunk -> | ||
| acc + chunk.content.length | ||
| } |
There was a problem hiding this comment.
maybe lazy so we dont have to calculate it multiple times
| fun supplementalContext(): SupplementalContextResult = if (supplementalContext != null) { | ||
| supplementalContext as SupplementalContextResult | ||
| } else { | ||
| runBlocking { |
There was a problem hiding this comment.
i think you should keep it as a Deferred, because the user-facing invocation is also under a coroutine
There was a problem hiding this comment.
okk,that makes sense to me. I think I do it this way because we log the sup context early or on failure. But will update this part
There was a problem hiding this comment.
just think of even after awaiting and getting the real sup context response within a coroutine, we still need a data class to store pure SupplemetalContextResult instead of its deferred?
There was a problem hiding this comment.
to solve that, should we use 2 different data class of RequestContext, one with Defer<SupplementalContext> and the other with real SupplementalConext ?
There was a problem hiding this comment.
oh richard i tried to restruct the flow and feel this should work
0a29d95
There was a problem hiding this comment.
should polish it a bit tho, wanna make getRequestContext fun signature still returns a regular object instead of defer but modify the call site
|
move unrelated refactor changes to #4806 |
Types of changes
Description
#4764
Checklist
License
I confirm that my contribution is made under the terms of the Apache 2.0 license.