Skip to content

Commit

Permalink
Merge branch 'googleapis:main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
kevkim-codes authored Dec 23, 2024
2 parents ecca0c4 + 8f9c355 commit 3a8f2f1
Show file tree
Hide file tree
Showing 19 changed files with 595 additions and 64 deletions.
1 change: 0 additions & 1 deletion .github/sync-repo-settings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ branchProtectionRules:
- test (18)
- cla/google
- windows
- OwlBot Post Processor
permissionRules:
- team: yoshi-admins
permission: admin
Expand Down
3 changes: 2 additions & 1 deletion owlbot.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@
source_location='build/src'
)
s.copy(templates,excludes=[
'.github/auto-approve.yml'
'.github/auto-approve.yml',
'.github/sync-repo-settings.yaml'
])

node.postprocess_gapic_library_hermetic()
44 changes: 40 additions & 4 deletions src/tabular-api-surface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,19 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`);
[]
);

/*
The following line of code sets the timeout if it was provided while
creating the client. This will be used to determine if the client should
retry on errors. Eventually, this will be handled downstream in google-gax.
*/
const timeout =
options?.gaxOptions?.timeout ||
(this?.bigtable?.options?.BigtableClient?.clientConfig?.interfaces &&
this?.bigtable?.options?.BigtableClient?.clientConfig?.interfaces[
'google.bigtable.v2.Bigtable'
]?.methods['MutateRows']?.timeout_millis);
const callTimeMillis = new Date().getTime();

let numRequestsMade = 0;

const maxRetries = is.number(this.maxRetries) ? this.maxRetries! : 3;
Expand All @@ -703,7 +716,14 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`);
);
const mutationErrorsByEntryIndex = new Map();

const isRetryable = (err: ServiceError | null) => {
const isRetryable = (
err: ServiceError | null,
timeoutExceeded: boolean
) => {
if (timeoutExceeded) {
// If the timeout has been exceeded then do not retry.
return false;
}
// Don't retry if there are no more entries or retry attempts
if (pendingEntryIndices.size === 0 || numRequestsMade >= maxRetries + 1) {
return false;
Expand All @@ -721,7 +741,10 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`);
return;
}

if (isRetryable(err)) {
const timeoutExceeded = !!(
timeout && timeout < new Date().getTime() - callTimeMillis
);
if (isRetryable(err, timeoutExceeded)) {
const backOffSettings =
options.gaxOptions?.retry?.backoffSettings ||
DEFAULT_BACKOFF_SETTINGS;
Expand All @@ -736,12 +759,25 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`);
err = null;
}

const mutationErrors = Array.from(mutationErrorsByEntryIndex.values());
if (mutationErrorsByEntryIndex.size !== 0) {
const mutationErrors = Array.from(mutationErrorsByEntryIndex.values());
callback(new PartialFailureError(mutationErrors, err));
return;
}

if (err) {
/* If there's an RPC level failure and the mutation entries don't have
a status code, the RPC level failure error code will be used as the
entry failure code.
*/
(err as ServiceError & {errors?: ServiceError[]}).errors =
mutationErrors.concat(
[...pendingEntryIndices]
.filter(index => !mutationErrorsByEntryIndex.has(index))
.map(() => err)
);
callback(err);
return;
}
callback(err);
};

Expand Down
4 changes: 2 additions & 2 deletions test/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,10 +494,10 @@ describe('Bigtable/Mutation', () => {
data: [],
};
const mutation = new Mutation(data);
Mutation.encodeSetCell = _data => {
sandbox.stub(Mutation, 'encodeSetCell').callsFake(_data => {
assert.strictEqual(_data, data.data);
return fakeEncoded;
};
});
const mutationProto = mutation.toProto();
assert.strictEqual(mutationProto.mutations, fakeEncoded);
assert.strictEqual(mutationProto.rowKey, data.key);
Expand Down
50 changes: 50 additions & 0 deletions test/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2752,6 +2752,56 @@ describe('Bigtable/Table', () => {
};
});

it('should send back errors for each pending entry', done => {
const mutateEntries = [
{
key: 'a',
method: 'insert',
data: {},
},
{
key: 'b',
method: 'insert',
data: {},
},
];
const unretriableError = new Error('not retryable') as ServiceError;
unretriableError.code = 3; // INVALID_ARGUMENT
emitters = [
((stream: Writable) => {
stream.emit('data', {
entries: [
{
index: 0,
status: {
details: [],
code: 0,
message: 'Received data',
},
},
],
});
stream.emit('error', unretriableError);
}) as {} as EventEmitter,
];
table.maxRetries = 3;
table.mutate(
mutateEntries,
(err: ServiceError & {errors?: ServiceError[]}) => {
try {
assert.strictEqual(err.code, 3);
assert.strictEqual(err.message, 'not retryable');
assert(err.errors);
assert.strictEqual(err.errors.length, 1);
assert.strictEqual(err.errors[0].code, 3);
assert.strictEqual(err.errors[0].message, 'not retryable');
done();
} catch (e) {
done(e);
}
}
);
});
it('should not retry unretriable errors', done => {
const unretriableError = new Error('not retryable') as ServiceError;
unretriableError.code = 3; // INVALID_ARGUMENT
Expand Down
159 changes: 159 additions & 0 deletions test/test-proxy/checkAndMutateRowService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import * as assert from 'assert';

import {describe} from 'mocha';
import {protos} from '../../src';
import {BigtableClient} from '../../src/v2';
import type {Callback, CallOptions, ServiceError} from 'google-gax';
const checkAndMutateRowService = require('../../../testproxy/services/check-and-mutate-row.js');
const createClient = require('../../../testproxy/services/create-client.js');

describe('TestProxy/CheckAndMutateRow', () => {
const testCases: protos.google.bigtable.v2.ICheckAndMutateRowRequest[] = [
{
tableName: 'projects/projectId/instances/instance/tables/test-table',
appProfileId: 'test-app-profile',
rowKey: Buffer.from('test-row-key'),
predicateFilter: null,
trueMutations: [
{
setCell: {
familyName: 'cf1',
timestampMicros: 1000007,
columnQualifier: Buffer.from('cq1'),
value: Buffer.from('value1'),
},
},
{
setCell: {
familyName: 'cf2',
timestampMicros: 1000007,
columnQualifier: Buffer.from('cq2'),
value: Buffer.from('value2'),
},
},
],
falseMutations: [
{
setCell: {
familyName: 'cf1',
timestampMicros: 1000007,
columnQualifier: Buffer.from('cq1'),
value: Buffer.from('value1'),
},
},
{
setCell: {
familyName: 'cf2',
timestampMicros: 1000007,
columnQualifier: Buffer.from('cq2'),
value: Buffer.from('value2'),
},
},
],
},
];
describe('Ensure the proper request is passed to the Gapic Layer', () => {
const clientId = 'TestCheckAndMutateRow_NoRetry_TransientError';
testCases.forEach((checkAndMutateRowRequest, index) => {
it(`Run test ${index}`, done => {
(async () => {
const clientMap = new Map();
const createClientFunction = createClient({clientMap});
await new Promise((resolve, reject) => {
createClientFunction(
{
request: {
clientId,
dataTarget: 'localhost:1234',
projectId: 'projectId',
instanceId: 'instance',
appProfileId: '',
},
},
(error: ServiceError, response: {}) => {
if (error) {
reject(error);
}
resolve(response);
}
);
});
{
// Mock out the Gapic layer so we can see requests coming into it
const bigtable = clientMap.get(clientId);
const bigtableClient = new BigtableClient(
bigtable.options.BigtableClient
);
bigtable.api['BigtableClient'] = bigtableClient;
bigtableClient.checkAndMutateRow = (
request?: protos.google.bigtable.v2.ICheckAndMutateRowRequest,
optionsOrCallback?:
| CallOptions
| Callback<
protos.google.bigtable.v2.ICheckAndMutateRowResponse,
| protos.google.bigtable.v2.ICheckAndMutateRowRequest
| null
| undefined,
{} | null | undefined
>,
callback?: Callback<
protos.google.bigtable.v2.ICheckAndMutateRowResponse,
| protos.google.bigtable.v2.ICheckAndMutateRowRequest
| null
| undefined,
{} | null | undefined
>
) => {
try {
// If the Gapic request is correct then the test passes.
assert.deepStrictEqual(request, checkAndMutateRowRequest);
} catch (e) {
// If the Gapic request is incorrect then the test fails with an error.
done(e);
}
if (callback) {
callback(null, {});
}
return new Promise(resolve => {
const response: protos.google.bigtable.v2.ICheckAndMutateRowResponse =
{};
resolve([response, {}, undefined]);
});
};
}
await new Promise((resolve, reject) => {
checkAndMutateRowService({clientMap})(
{
request: {
clientId,
request: checkAndMutateRowRequest,
},
},
(error: ServiceError, response: {}) => {
if (error) {
reject(error);
}
resolve(response);
}
);
});
done();
})();
});
});
});
});
46 changes: 46 additions & 0 deletions test/test-proxy/mutationParseInverse.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2024 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import * as assert from 'assert';
import {Mutation} from '../../src/mutation';
import {mutationParseInverse} from '../../testproxy/services/utils/request/mutateInverse';

describe('Check mutation parse and mutationParseInverse are inverses', () => {
it('should invert mutations properly', () => {
const gapicLayerRequest = {
mutations: [
{
setCell: {
familyName: 'cf1',
timestampMicros: 1000007,
columnQualifier: Buffer.from('cq1'),
value: Buffer.from('value1'),
},
},
{
setCell: {
familyName: 'cf2',
timestampMicros: 1000007,
columnQualifier: Buffer.from('cq2'),
value: Buffer.from('value2'),
},
},
],
};
assert.deepStrictEqual(
Mutation.parse(mutationParseInverse(gapicLayerRequest)),
gapicLayerRequest
);
});
});
Loading

0 comments on commit 3a8f2f1

Please sign in to comment.