-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Cosmos] Simple endpoint refresh interval #15781
Changes from 9 commits
5325db5
99b243a
2615ae1
4fb8695
ca1a7a7
9934022
b7f00dc
e721e59
e5610c8
5c5bec6
275eafc
47ba254
bfaa965
3f60424
97fb7b1
303cd17
8241909
a3ffe51
2038734
57e3d6b
19812c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,10 @@ export interface ConnectionPolicy { | |
connectionMode?: ConnectionMode; | ||
/** Request timeout (time to wait for response from network peer). Represented in milliseconds. */ | ||
requestTimeout?: number; | ||
/** Flag to enable/disable automatic redirecting of requests based on read/write operations. */ | ||
/** | ||
* Flag to enable/disable automatic redirecting of requests based on read/write operations. | ||
* Required to call client.dispose() when this is set to true after destroying the CosmosClient inside another process or in the browser. | ||
zfoster marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
enableEndpointDiscovery?: boolean; | ||
/** List of azure regions to be used as preferred locations for read requests. */ | ||
preferredLocations?: string[]; | ||
|
@@ -21,16 +24,23 @@ export interface ConnectionPolicy { | |
* Default is `false`. | ||
*/ | ||
useMultipleWriteLocations?: boolean; | ||
/** Rate in milliseconds at which the client will refresh the endpoints list in the background */ | ||
endpointRefreshRateInMs?: number; | ||
} | ||
|
||
/** | ||
* @hidden | ||
*/ | ||
export const defaultConnectionPolicy = Object.freeze({ | ||
export const defaultConnectionPolicy: ConnectionPolicy = Object.freeze({ | ||
connectionMode: ConnectionMode.Gateway, | ||
requestTimeout: 60000, | ||
enableEndpointDiscovery: true, | ||
preferredLocations: [], | ||
retryOptions: {}, | ||
useMultipleWriteLocations: true | ||
retryOptions: { | ||
maxRetryAttemptCount: 9, | ||
fixedRetryIntervalInMilliseconds: 100, | ||
maxWaitTimeInSeconds: 30 | ||
}, | ||
useMultipleWriteLocations: true, | ||
endpointRefreshRateInMs: 300000 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a breaking change? Just want to verify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a default on the |
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,8 +25,8 @@ export class GlobalEndpointManager { | |
* List of azure regions to be used as preferred locations for read requests. | ||
*/ | ||
private preferredLocations: string[]; | ||
private writeableLocations: Location[]; | ||
private readableLocations: Location[]; | ||
private writeableLocations: Location[] = []; | ||
private readableLocations: Location[] = []; | ||
|
||
/** | ||
* @param options - The document client instance. | ||
|
@@ -175,7 +175,7 @@ export class GlobalEndpointManager { | |
this.writeableLocations.push(location); | ||
} | ||
} | ||
for (const location of databaseAccount.writableLocations) { | ||
for (const location of databaseAccount.readableLocations) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @southpolesteve do you think this was a bug? this causes test failures There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure. Did this method work before this change? Let's discuss tomorrow |
||
const existingLocation = this.readableLocations.find((loc) => loc.name === location.name); | ||
if (!existingLocation) { | ||
this.readableLocations.push(location); | ||
|
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 should make sure the ref docs for
enableEndpointDiscovery
talk about the requirement to calldispose