Skip to content

Commit

Permalink
fix: do not throw DeprecationWarnings for legacy retry behavior (#1551)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexander-fenster
alexander-fenster authored Jan 31, 2024
1 parent 3efb193 commit 2f39306
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 179 deletions.
14 changes: 0 additions & 14 deletions gax/src/gax.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,20 +333,6 @@ export function convertRetryOptions(
// if a user provided retry AND retryRequestOptions at call time, throw an error
// otherwise, convert supported parameters
if (!gaxStreamingRetries) {
if (options.retry) {
warn(
'legacy_streaming_retry_behavior',
'Legacy streaming retry behavior will not honor settings passed at call time or via client configuration. Please set gaxStreamingRetries to true to utilize passed retry settings. gaxStreamingRetries behavior will be set to true by default in future releases.',
'DeprecationWarning'
);
}
if (options.retryRequestOptions) {
warn(
'legacy_streaming_retry_request_behavior',
'Legacy streaming retry behavior will not honor retryRequestOptions passed at call time. Please set gaxStreamingRetries to true to utilize passed retry settings. gaxStreamingRetries behavior will convert retryRequestOptions to retry parameters by default in future releases.',
'DeprecationWarning'
);
}
return options;
}
if (options.retry && options.retryRequestOptions) {
Expand Down
165 changes: 0 additions & 165 deletions gax/test/unit/streaming.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ describe('streaming', () => {
});

it('cancels in the middle', done => {
const warnStub = sinon.stub(warnings, 'warn');

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function schedulePush(s: any, c: number) {
const intervalId = setInterval(() => {
Expand Down Expand Up @@ -295,14 +293,6 @@ describe('streaming', () => {
assert.strictEqual(err, cancelError);
done();
});
assert.strictEqual(warnStub.callCount, 1);
assert(
warnStub.calledWith(
'legacy_streaming_retry_behavior',
'Legacy streaming retry behavior will not honor settings passed at call time or via client configuration. Please set gaxStreamingRetries to true to utilize passed retry settings. gaxStreamingRetries behavior will be set to true by default in future releases.',
'DeprecationWarning'
)
);
});

it('emit response when stream received metadata event', done => {
Expand Down Expand Up @@ -598,8 +588,6 @@ describe('streaming', () => {
});

it('emit parsed GoogleError', done => {
const warnStub = sinon.stub(warnings, 'warn');

const errorInfoObj = {
reason: 'SERVICE_DISABLED',
domain: 'googleapis.com',
Expand Down Expand Up @@ -676,14 +664,6 @@ describe('streaming', () => {
s.on('end', () => {
done();
});
assert.strictEqual(warnStub.callCount, 1);
assert(
warnStub.calledWith(
'legacy_streaming_retry_behavior',
'Legacy streaming retry behavior will not honor settings passed at call time or via client configuration. Please set gaxStreamingRetries to true to utilize passed retry settings. gaxStreamingRetries behavior will be set to true by default in future releases.',
'DeprecationWarning'
)
);
});
it('emit parsed GoogleError when new retries are enabled', done => {
const errorInfoObj = {
Expand Down Expand Up @@ -1982,101 +1962,6 @@ describe('warns/errors about server streaming retry behavior when gaxStreamingRe
sinon.restore();
});

it('throws a warning when retryRequestOptions are passed', done => {
const warnStub = sinon.stub(warnings, 'warn');

// this exists to help resolve createApiCall
sinon.stub(StreamingApiCaller.prototype, 'call').callsFake(() => {
done();
});

const spy = sinon.spy((...args: Array<{}>) => {
assert.strictEqual(args.length, 3);
const s = new PassThrough({
objectMode: true,
});
return s;
});

const apiCall = createApiCallStreaming(
spy,
streaming.StreamType.SERVER_STREAMING,
false,
false // ensure we are NOT opted into the new retry behavior
);
const passedRetryRequestOptions = {
objectMode: false,
retries: 1,
maxRetryDelay: 70,
retryDelayMultiplier: 3,
totalTimeout: 650,
noResponseRetries: 3,
currentRetryAttempt: 0,
shouldRetryFn: function alwaysRetry() {
return true;
},
};
// make the call with both options passed at call time
apiCall(
{},
{
retryRequestOptions: passedRetryRequestOptions,
}
);
assert.strictEqual(warnStub.callCount, 1);
assert(
warnStub.calledWith(
'legacy_streaming_retry_request_behavior',
'Legacy streaming retry behavior will not honor retryRequestOptions passed at call time. Please set gaxStreamingRetries to true to utilize passed retry settings. gaxStreamingRetries behavior will convert retryRequestOptions to retry parameters by default in future releases.',
'DeprecationWarning'
)
);
});
it('throws a warning when retry options are passed', done => {
const warnStub = sinon.stub(warnings, 'warn');
// this exists to help resolve createApiCall
sinon.stub(StreamingApiCaller.prototype, 'call').callsFake(() => {
done();
});

const spy = sinon.spy((...args: Array<{}>) => {
assert.strictEqual(args.length, 3);
const s = new PassThrough({
objectMode: true,
});
return s;
});

const apiCall = createApiCallStreaming(
spy,
streaming.StreamType.SERVER_STREAMING,
false,
false // ensure we are NOT opted into the new retry behavior
);

// make the call with both options passed at call time
apiCall(
{},
{
retry: gax.createRetryOptions([1], {
initialRetryDelayMillis: 300,
retryDelayMultiplier: 1.2,
maxRetryDelayMillis: 1000,
rpcTimeoutMultiplier: 1.5,
maxRpcTimeoutMillis: 3000,
totalTimeoutMillis: 4500,
}),
}
);
assert.strictEqual(warnStub.callCount, 1);
assert(
warnStub.calledWith(
'legacy_streaming_retry_behavior',
'Legacy streaming retry behavior will not honor settings passed at call time or via client configuration. Please set gaxStreamingRetries to true to utilize passed retry settings. gaxStreamingRetries behavior will be set to true by default in future releases.',
'DeprecationWarning'
)
);
});
it('throws no warnings when when no retry options are passed', done => {
const warnStub = sinon.stub(warnings, 'warn');
// this exists to help resolve createApiCall
Expand All @@ -2103,56 +1988,6 @@ describe('warns/errors about server streaming retry behavior when gaxStreamingRe
apiCall({}, {});
assert.strictEqual(warnStub.callCount, 0);
});
it('throws two warnings when when retry and retryRequestoptions are passed', done => {
const warnStub = sinon.stub(warnings, 'warn');
// this exists to help resolve createApiCall
sinon.stub(StreamingApiCaller.prototype, 'call').callsFake(() => {
done();
});

const spy = sinon.spy((...args: Array<{}>) => {
assert.strictEqual(args.length, 3);
const s = new PassThrough({
objectMode: true,
});
return s;
});

const apiCall = createApiCallStreaming(
spy,
streaming.StreamType.SERVER_STREAMING,
false,
false // ensure we are NOT opted into the new retry behavior
);
const passedRetryRequestOptions = {
objectMode: false,
retries: 1,
maxRetryDelay: 70,
retryDelayMultiplier: 3,
totalTimeout: 650,
noResponseRetries: 3,
currentRetryAttempt: 0,
shouldRetryFn: function alwaysRetry() {
return true;
},
};
// make the call with both retry options passed at call time
apiCall(
{},
{
retryRequestOptions: passedRetryRequestOptions,
retry: gax.createRetryOptions([1], {
initialRetryDelayMillis: 300,
retryDelayMultiplier: 1.2,
maxRetryDelayMillis: 1000,
rpcTimeoutMultiplier: 1.5,
maxRpcTimeoutMillis: 3000,
totalTimeoutMillis: 4500,
}),
}
);
assert.strictEqual(warnStub.callCount, 2);
});
});

describe('REST streaming apiCall return StreamArrayParser', () => {
Expand Down

0 comments on commit 2f39306

Please sign in to comment.