Skip to content
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

feat: add support for spanner client resource base routing #780

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
3b5c4de
feat: added method to get endPointUris and related changes
Dec 5, 2019
241fe62
fix: unit test cases was failing
Dec 6, 2019
d4d50b7
pass instanceId externally in each SpannerClient Request
Dec 6, 2019
87fd04a
feat: added flag to check weather resource based routing is enable or…
Dec 6, 2019
1ba9e16
move getInstanceEndPointUris to instance.ts file
Dec 9, 2019
2ac95bf
test: added unit test cases
Dec 10, 2019
f504666
fix: removed end point uris mapping chaching
Dec 11, 2019
cc5e72f
test:added more unit test cases
Dec 11, 2019
dfb8100
provided an option to enable resource based routing to user
Dec 13, 2019
9d4f4af
pass the instance id insted of formmatedName_ in each method
Dec 17, 2019
67f74af
fix: unit test cases
Dec 17, 2019
15f261d
fix: unit test cases
Dec 17, 2019
fe0e1a5
test:added system test
Dec 19, 2019
d46a13c
test:added unit test for instance permission
Dec 30, 2019
c4c12d2
fix:review changes
Dec 31, 2019
aae7311
refactor: simplify and clean up
Dec 31, 2019
0b7e83e
fix: reverse the resource based routing condition
Jan 1, 2020
59cdb4f
test: added unit test for multiple endpointUri
Jan 2, 2020
06fba33
Merge branch 'master' into resource-based-routing
Jan 10, 2020
600d5f1
fix:correct spell
Jan 10, 2020
2c99834
Merge branch 'master' into resource-based-routing
Jan 16, 2020
846cb78
refactor: replace getInstanceEndpointUris with getMetadata
Jan 16, 2020
1e83f37
Merge branch 'master' into resource-based-routing
Jan 22, 2020
57687ba
Merge branch 'master' into resource-based-routing
AVaksman Jan 23, 2020
eefc95d
Merge branch 'master' into resource-based-routing
Jan 24, 2020
4c2f6aa
fix:review changes
Jan 24, 2020
48085fb
fix: unit-test case fail issue
Jan 24, 2020
eb358f3
refactor: added enum to check the permission status
Jan 27, 2020
06dc72d
Merge branch 'master' into resource-based-routing
Jan 28, 2020
e5b02c5
merge:merge from master
Feb 6, 2020
710bc9a
fix: review changes
Feb 6, 2020
66eac38
Merge branch 'master' into resource-based-routing
Feb 10, 2020
a464043
test: added unit test and system-test cases
Feb 12, 2020
f3b976c
Merge branch 'master' into resource-based-routing
Feb 12, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 44 additions & 6 deletions system-test/spanner.ts
Expand Up @@ -4420,12 +4420,50 @@ describe('Spanner', () => {
spanner.options.enableResourceBasedRouting = false;
});

it('should list the session while resource based routing is enabled', done => {
database.getSessions((err, session) => {
assert.ifError(err);
assert.equal(session!.length, 0);
done();
});
it('should use resolved endpoint while resource based routing is enabled', done => {
instance.getMetadata(
{fieldNames: ['endpointUris']},
(err, instanceMetadata) => {
assert.ifError(err);
// tslint:disable-next-line: no-any
const resolvedEndpoint: string[] = (instanceMetadata as any)
.endpointUris;
if (resolvedEndpoint.length === 0) {
done(); //no resolved endpoint.
}
database.getSessions((err, session) => {
assert.ifError(err);
assert.strictEqual(
spanner.clients_.has(`SpannerClient-${instance.id}`),
true
);
done();
});
}
);
});

it('should use user-specified endpoint when resource based routing is enabled.', done => {
instance.getMetadata(
{fieldNames: ['endpointUris']},
(err, instanceMetadata) => {
assert.ifError(err);
// tslint:disable-next-line: no-any
const resolvedEndpoint: string[] = (instanceMetadata as any)
.endpointUris;
if (resolvedEndpoint.length === 0) {
done(); //no resolved endpoint.
}
//user specific apiEndpoint
spanner.options.apiEndpoint = resolvedEndpoint[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this using the resolved endpoint instead of a user-specified endpoint?

spanner.options.enableResourceBasedRouting = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think at line 4417, resource routing is already enabled so don't think you need this.

database.getSessions((err, session) => {
assert.ifError(err);
assert.strictEqual(spanner.clients_.has('SpannerClient'), true);
done();
});
}
);
});
});
});
Expand Down
8 changes: 2 additions & 6 deletions test/index.ts
Expand Up @@ -1039,17 +1039,13 @@ describe('Spanner', () => {
spanner.prepareGapicRequest_(CONFIG, assert.ifError);
});

it('should use user-specified endpoint when GetInstance response is empty.', done => {
it('should use user-specified endpoint when resource based routing is enabled.', done => {
const instanceId = 'instance-id';
const customEndpoint = 'us-central1-spanner.googleapis.com';
asAny(CONFIG).instanceId = instanceId;
asAny(spanner.options).apiEndpoint = customEndpoint;
spanner.options.enableResourceBasedRouting = true;

asAny(spanner).instance(instanceId).getMetadata = (options, callback) => {
asAny(spanner.options).apiEndpoint = customEndpoint;
callback!(null, {endpointUris: []});
};

fakeV1[CONFIG.client] = class {
constructor(options) {
assert.equal(
Expand Down