Skip to content

Commit

Permalink
[core-rest-pipeline] Flip keepAlive boolean (Azure#14093)
Browse files Browse the repository at this point in the history
Booleans should default to false, but keepAlive on PipelineRequest defaulted to true. This change replaces `keepAlive` with `disableKeepAlive`, which can default to false.
  • Loading branch information
xirzec authored Mar 4, 2021
1 parent 9438fb2 commit 68a0144
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 12 deletions.
4 changes: 2 additions & 2 deletions sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ export interface PipelinePolicy {
export interface PipelineRequest {
abortSignal?: AbortSignalLike;
body?: RequestBodyType;
disableKeepAlive?: boolean;
formData?: FormDataMap;
headers: HttpHeaders;
keepAlive?: boolean;
method: HttpMethods;
onDownloadProgress?: (progress: TransferProgressEvent) => void;
onUploadProgress?: (progress: TransferProgressEvent) => void;
Expand All @@ -173,9 +173,9 @@ export interface PipelineRequest {
export interface PipelineRequestOptions {
abortSignal?: AbortSignalLike;
body?: RequestBodyType;
disableKeepAlive?: boolean;
formData?: FormDataMap;
headers?: HttpHeaders;
keepAlive?: boolean;
method?: HttpMethods;
onDownloadProgress?: (progress: TransferProgressEvent) => void;
onUploadProgress?: (progress: TransferProgressEvent) => void;
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/core-rest-pipeline/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ export interface PipelineRequest {
proxySettings?: ProxySettings;

/**
* If the connection should be reused.
* If the connection should not be reused.
*/
keepAlive?: boolean;
disableKeepAlive?: boolean;

/**
* Used to abort the request later.
Expand Down
6 changes: 4 additions & 2 deletions sdk/core/core-rest-pipeline/src/nodeHttpsClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ class NodeHttpsClient implements HttpsClient {
try {
parsedUrl = new URL(proxySettings.host);
} catch (_error) {
throw new Error(`Expecting a valid host string in proxy settings, but found "${proxySettings.host}".`);
throw new Error(
`Expecting a valid host string in proxy settings, but found "${proxySettings.host}".`
);
}

const proxyAgentOptions: HttpsProxyAgentOptions = {
Expand All @@ -217,7 +219,7 @@ class NodeHttpsClient implements HttpsClient {
this.proxyAgent = (new HttpsProxyAgent(proxyAgentOptions) as unknown) as https.Agent;
}
return this.proxyAgent;
} else if (request.keepAlive) {
} else if (!request.disableKeepAlive) {
if (!this.keepAliveAgent) {
this.keepAliveAgent = new https.Agent({
keepAlive: true
Expand Down
8 changes: 4 additions & 4 deletions sdk/core/core-rest-pipeline/src/pipelineRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ export interface PipelineRequestOptions {
proxySettings?: ProxySettings;

/**
* If the connection should be reused. Defaults to true.
* If the connection should not be reused.
*/
keepAlive?: boolean;
disableKeepAlive?: boolean;

/**
* Used to abort the request later.
Expand Down Expand Up @@ -107,7 +107,7 @@ class PipelineRequestImpl implements PipelineRequest {
public formData?: FormDataMap;
public streamResponseStatusCodes?: Set<number>;
public proxySettings?: ProxySettings;
public keepAlive: boolean;
public disableKeepAlive: boolean;
public abortSignal?: AbortSignalLike;
public requestId: string;
public spanOptions?: SpanOptions;
Expand All @@ -121,7 +121,7 @@ class PipelineRequestImpl implements PipelineRequest {
this.method = options.method ?? "GET";
this.timeout = options.timeout ?? 0;
this.formData = options.formData;
this.keepAlive = options.keepAlive ?? true;
this.disableKeepAlive = options.disableKeepAlive ?? false;
this.proxySettings = options.proxySettings;
this.streamResponseStatusCodes = options.streamResponseStatusCodes;
this.withCredentials = options.withCredentials ?? false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ export class CoreHTTPSDownloadWithSASTest extends StorageBlobDownloadWithSASTest
this.client = createDefaultHttpsClient();
this.request = createPipelineRequest({
url: this.sasUrl,
streamResponseStatusCodes: new Set([200, 206]),
keepAlive: true
streamResponseStatusCodes: new Set([200, 206])
});
}

Expand Down

0 comments on commit 68a0144

Please sign in to comment.