Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 143 additions & 0 deletions packages/core/src/codewhisperer/region/regionProfileManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { parse } from '@aws-sdk/util-arn-parser'
import { isAwsError, ToolkitError } from '../../shared/errors'
import { telemetry } from '../../shared/telemetry/telemetry'
import { localize } from '../../shared/utilities/vsCodeUtils'
import { waitUntil } from '../../shared/utilities/timeoutUtils'

// TODO: is there a better way to manage all endpoint strings in one place?
export const defaultServiceConfig: CodeWhispererConfig = {
Expand All @@ -49,6 +50,33 @@ const endpoints = createConstantMap({
*/
export type ProfileSwitchIntent = 'user' | 'auth' | 'update' | 'reload'

type CachedApiResultWithLock = {
// Lock
locked: boolean
// Metadata
result: SuccessResult | FailureResult | undefined
}

interface ApiResult {
type: 'success' | 'failure'
timestamp: number
}

interface SuccessResult extends ApiResult {
type: 'success'
profiles: RegionProfile[]
}

interface FailureResult extends ApiResult {
type: 'failure'
// Error type can't be serialized
errorMsg: string
}

function now(): number {
return globals.clock.Date.now()
}

export class RegionProfileManager {
private static logger = getLogger()
private _activeRegionProfile: RegionProfile | undefined
Expand Down Expand Up @@ -110,7 +138,43 @@ export class RegionProfileManager {
if (conn === undefined || !isSsoConnection(conn)) {
return []
}

const cached = await this.acquireLock()

RegionProfileManager.logger.info(`obtained cache lock %s`, cached)

const availableProfiles: RegionProfile[] = []
const now = globals.clock.Date.now()
if (cached?.result && now - cached.result.timestamp < 60000) {
RegionProfileManager.logger.info(
`cache hit, duration=%s, previous listAvailableProfile result=%s`,
now - cached.result.timestamp,
cached
)

// Release lock so that other ide instances can continue
await this.releaesLock(cached)

if (cached.result.type === 'success') {
availableProfiles.push(...(cached.result as SuccessResult).profiles)
this._profiles = availableProfiles
return availableProfiles
} else if (cached.result.type === 'failure') {
const errorMsg = (cached.result as FailureResult).errorMsg
throw new Error(errorMsg)
}
}

if (cached?.result && now - cached.result.timestamp > 60000) {
RegionProfileManager.logger.info(
`listAvailableProfile cache hit but cached value is stale, invoking service API to pull the latest response`
)
} else {
RegionProfileManager.logger.info(
`listAvailableProfile cache miss, invoking service API to pull the latest response`
)
}

for (const [region, endpoint] of endpoints.entries()) {
const client = await this.createQClient(region, endpoint, conn as SsoConnection)
const requester = async (request: CodeWhispererUserClient.ListAvailableProfilesRequest) =>
Expand Down Expand Up @@ -138,12 +202,17 @@ export class RegionProfileManager {
} catch (e) {
const logMsg = isAwsError(e) ? `requestId=${e.requestId}; message=${e.message}` : (e as Error).message
RegionProfileManager.logger.error(`failed to listRegionProfile: ${logMsg}`)
// Should update cache even for failure path
await this.updateCacheWithError(e as Error)
throw e
}

RegionProfileManager.logger.info(`available amazonq profiles: ${availableProfiles.length}`)
}

// Update cache and save the result
await this.updateCacheWithResult(availableProfiles)

this._profiles = availableProfiles
return availableProfiles
}
Expand Down Expand Up @@ -367,4 +436,78 @@ export class RegionProfileManager {

return c
}

// Should reset cache when users signout in case users want to change to a different connection
async resetCache() {
await globals.globalState.update('aws.amazonq.regionProfiles.cachedResult', {
isAcquired: false,
result: undefined,
})
}

private async acquireLock(): Promise<CachedApiResultWithLock | undefined> {
async function _acquireLock() {
const cachedValue = globals.globalState.tryGet<CachedApiResultWithLock>(
'aws.amazonq.regionProfiles.cachedResult',
Object,
{ locked: false, result: undefined }
)

if (!cachedValue.locked) {
await globals.globalState.update('aws.amazonq.regionProfiles.cachedResult', {
...cachedValue,
isAcquired: true,
})

return cachedValue
}
return undefined
}

const lock = await waitUntil(
async () => {
const lock = await _acquireLock()
RegionProfileManager.logger.info(`try obtaining cache lock %s`, lock)
if (lock) {
return lock
}
},
{ timeout: 15000, interval: 1500, truthy: true }
)

return lock
}

private async releaesLock(cached: CachedApiResultWithLock) {
await globals.globalState.update('aws.amazonq.regionProfiles.cachedResult', {
...cached,
isAcquired: false,
})
}

private async updateCacheWithResult(r: RegionProfile[]) {
const result: SuccessResult = {
type: 'success',
profiles: r,
timestamp: now(),
}
const pojo: CachedApiResultWithLock = {
locked: false, // release the lock
result: result,
}
await globals.globalState.update('aws.amazonq.regionProfiles.cachedResult', pojo)
}

private async updateCacheWithError(e: Error) {
const result: FailureResult = {
type: 'failure',
errorMsg: e.message,
timestamp: now(),
}
const pojo: CachedApiResultWithLock = {
locked: false, // release the lock
result: result,
}
await globals.globalState.update('aws.amazonq.regionProfiles.cachedResult', pojo)
}
}
1 change: 1 addition & 0 deletions packages/core/src/codewhisperer/util/authUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export class AuthUtil {

if (!this.isConnected()) {
await this.regionProfileManager.invalidateProfile(this.regionProfileManager.activeRegionProfile?.arn)
await this.regionProfileManager.resetCache()
}
})

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/shared/globalState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export type globalKey =
| 'aws.toolkit.lsp.manifest'
| 'aws.amazonq.customization.overrideV2'
| 'aws.amazonq.regionProfiles'
| 'aws.amazonq.regionProfiles.cachedResult'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not set a flag on the items in aws.amazonq.regionProfiles ? why does this need an entirely separate structure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'aws.amazonq.regionProfiles'

this is to memorize previous selected option, so we will need a separate structure to cache previous api call response

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'aws.amazonq.regionProfiles'

this is to memorize previous selected option

but that could be a flag on the item(s) in the structure. why is a separate structure needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense, i will reuse the existing one then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinmk3 i encounter an issue or say concern when i am doing this exercise 3e50145#diff-f5716036a70a8b212e9dae829ec09e33731f181c173a3f2b9ad9d361d51af14bR54-R56, if I want to reuse the same global key, i might need to restructure the schema, in such case, users who already selected profile update their plugin, they might need to re-select the profile again. Given the sev2s we have right now, I am concerned if we want to do this now or is there another way to deal with such scenario?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely want to avoid a restructure. If a restructure is required, then I guess we need a new globalState field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted

// Deprecated/legacy names. New keys should start with "aws.".
| '#sessionCreationDates' // Legacy name from `ssoAccessTokenProvider.ts`.
| 'CODECATALYST_RECONNECT'
Expand Down