diff --git a/src/cluster.ts b/src/cluster.ts index 4f06d1808..44906383f 100644 --- a/src/cluster.ts +++ b/src/cluster.ts @@ -59,16 +59,21 @@ export type GetClustersCallback = ( clusters?: Cluster[], apiResponse?: google.bigtable.admin.v2.IListClustersResponse ) => void; +export interface SetClusterMetadataOptions { + nodes: number; +} export type SetClusterMetadataCallback = GenericOperationCallback< Operation | null | undefined >; - -export interface CreateClusterOptions { - gaxOptions?: CallOptions; - location?: string; +export interface BasicClusterConfig { + location: string; nodes: number; storage?: string; } + +export interface CreateClusterOptions extends BasicClusterConfig { + gaxOptions?: CallOptions; +} export type GetClusterMetadataCallback = ( err: ServiceError | null, metadata?: ICluster | null, @@ -367,18 +372,15 @@ Please use the format 'my-cluster' or '${instance.name}/clusters/my-cluster'.`); } setMetadata( - metadata: CreateClusterOptions + metadata: SetClusterMetadataOptions, + gaxOptions?: CallOptions ): Promise; setMetadata( - metadata: CreateClusterOptions, - gaxOptions: CallOptions - ): Promise; - setMetadata( - metadata: CreateClusterOptions, + metadata: SetClusterMetadataOptions, callback: SetClusterMetadataCallback ): void; setMetadata( - metadata: CreateClusterOptions, + metadata: SetClusterMetadataOptions, gaxOptions: CallOptions, callback: SetClusterMetadataCallback ): void; @@ -399,35 +401,27 @@ Please use the format 'my-cluster' or '${instance.name}/clusters/my-cluster'.`); * region_tag:bigtable_cluster_set_meta */ setMetadata( - metadata: CreateClusterOptions, + metadata: SetClusterMetadataOptions, gaxOptionsOrCallback?: CallOptions | SetClusterMetadataCallback, cb?: SetClusterMetadataCallback ): void | Promise { const callback = typeof gaxOptionsOrCallback === 'function' ? gaxOptionsOrCallback : cb!; const gaxOptions = - typeof gaxOptionsOrCallback === 'object' && gaxOptionsOrCallback + typeof gaxOptionsOrCallback === 'object' ? gaxOptionsOrCallback : ({} as CallOptions); - const reqOpts: ICluster = { - name: this.name, - }; - - if (metadata.location) { - reqOpts.location = Cluster.getLocation_( - this.bigtable.projectId, - metadata.location - ); - } - - if (metadata.nodes) { - reqOpts.serveNodes = metadata.nodes; - } - - if (metadata.storage) { - reqOpts.defaultStorageType = Cluster.getStorageType_(metadata.storage); - } + const reqOpts: ICluster = Object.assign( + {}, + { + name: this.name, + serveNodes: metadata.nodes, + }, + metadata + ); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + delete (reqOpts as any).nodes; this.bigtable.request( { diff --git a/src/index.ts b/src/index.ts index f15779281..2b6f54f11 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,7 +27,6 @@ import { InstanceOptions, CreateInstanceCallback, CreateInstanceResponse, - ClusterInfo, IInstance, } from './instance'; import {shouldRetryRequest} from './decorateStatus'; @@ -49,14 +48,14 @@ const {grpc} = new gax.GrpcClient(); export interface GetInstancesCallback { ( err: ServiceError | null, - result?: Instance[] | null, - nextQuery?: {} | null, - response?: google.bigtable.admin.v2.IListInstancesResponse | null + result?: Instance[], + failedLocations?: string[], + response?: google.bigtable.admin.v2.IListInstancesResponse ): void; } export type GetInstancesResponse = [ Instance[], - {} | null, + string[], google.bigtable.admin.v2.IListInstancesResponse ]; @@ -462,14 +461,13 @@ export class Bigtable { createInstance( id: string, - options?: InstanceOptions + options: InstanceOptions ): Promise; createInstance( id: string, options: InstanceOptions, callback: CreateInstanceCallback ): void; - createInstance(id: string, callback: CreateInstanceCallback): void; /** * Create a Cloud Bigtable instance. * @@ -479,7 +477,7 @@ export class Bigtable { * @param {object} options Instance creation options. * @param {object[]} options.clusters The clusters to be created within the * instance. - * @param {string} options.displayName The descriptive name for this instance + * @param {string} [options.displayName] The descriptive name for this instance * as it appears in UIs. * @param {Object.} [options.labels] Labels are a flexible and * lightweight mechanism for organizing cloud resources into groups that @@ -546,14 +544,19 @@ export class Bigtable { */ createInstance( id: string, - optionsOrCallback?: InstanceOptions | CreateInstanceCallback, - cb?: CreateInstanceCallback + options: InstanceOptions, + callback?: CreateInstanceCallback ): void | Promise { - const options = - typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; - const callback = - typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; - + if (typeof options !== 'object') { + throw new Error( + 'A configuration object is required to create an instance.' + ); + } + if (!options.clusters) { + throw new Error( + 'At least one cluster configuration object is required to create an instance.' + ); + } const reqOpts = { parent: this.projectName, instanceId: id, @@ -567,7 +570,7 @@ export class Bigtable { reqOpts.instance!.type = Instance.getTypeType_(options.type); } - reqOpts.clusters = arrify(options.clusters!).reduce((clusters, cluster) => { + reqOpts.clusters = arrify(options.clusters).reduce((clusters, cluster) => { if (!cluster.id) { throw new Error( 'A cluster was provided without an `id` property defined.' @@ -581,7 +584,7 @@ export class Bigtable { }; return clusters; - }, {} as {[index: string]: ClusterInfo}); + }, {} as {[index: string]: google.bigtable.admin.v2.ICluster}); this.request( { @@ -606,12 +609,14 @@ export class Bigtable { /** * @typedef {array} GetInstancesResponse * @property {Instance[]} 0 Array of {@link Instance} instances. - * @property {object} 1 The full API response. + * @property {string[]} 1 locations from which Instance information could not be retrieved + * @property {object} 2 The full API response. */ /** * @callback GetInstancesCallback * @param {?Error} err Request error, if any. * @param {Instance[]} instances Array of {@link Instance} instances. + * @param {string[]} locations from which Instance information could not be retrieved * @param {object} apiResponse The full API response. */ /** @@ -629,26 +634,21 @@ export class Bigtable { * bigtable.getInstances(function(err, instances) { * if (!err) { * // `instances` is an array of Instance objects. + * if (failedLocations.length > 0) { + * // These locations contain instances which could not be retrieved. + * } * } * }); * - * @example To control how many API requests are made and page - * through the results manually, set `autoPaginate` to `false`. - * function callback(err, instances, nextQuery, apiResponse) { - * if (nextQuery) { - * // More results exist. - * bigtable.getInstances(nextQuery, callback); - * } - * } - * - * bigtable.getInstances({ - * autoPaginate: false - * }, callback); - * * @example If the callback is omitted, we'll return a Promise. * * bigtable.getInstances().then(function(data) { * const instances = data[0]; + * + * if (data[1]) { + * // These locations contain instances which could not be retrieved. + * const failedLocations = data[1]; + * } * }); */ getInstances( @@ -683,7 +683,7 @@ export class Bigtable { instance.metadata = instanceData; return instance; }); - callback!(null, instances, resp); + callback!(null, instances, resp.failedLocations, resp); } ); } diff --git a/src/instance.ts b/src/instance.ts index 8c4768483..b5e53275e 100644 --- a/src/instance.ts +++ b/src/instance.ts @@ -32,6 +32,8 @@ import { CreateClusterResponse, GetClustersCallback, GetClustersResponse, + IOperation, + BasicClusterConfig, } from './cluster'; import {Family} from './family'; import { @@ -56,20 +58,15 @@ import {ServiceError} from 'google-gax'; import {Bigtable} from '.'; import {google} from '../protos/protos'; -export interface ClusterInfo { - id?: string; - location?: string; - serveNodes?: number; - nodes?: number; - storage?: string; - defaultStorageType?: number; +export interface ClusterInfo extends BasicClusterConfig { + id: string; } export interface InstanceOptions { /** * The clusters to be created within the instance. */ - clusters?: ClusterInfo[] | ClusterInfo; + clusters: ClusterInfo[] | ClusterInfo; /** * The descriptive name for this instance as it appears in UIs. @@ -102,13 +99,16 @@ export interface InstanceOptions { } export type IInstance = google.bigtable.admin.v2.IInstance; -export type CreateInstanceCallback = ( - err: ServiceError | null, - instance?: Instance, - operation?: Operation, - apiResponse?: IInstance -) => void; -export type CreateInstanceResponse = [Instance, Operation, IInstance]; +export interface LongRunningResourceCallback { + ( + err: ServiceError | null, + resource?: Resource, + operation?: Operation, + apiResponse?: IOperation + ): void; +} +export type CreateInstanceCallback = LongRunningResourceCallback; +export type CreateInstanceResponse = [Instance, Operation, IOperation]; export type DeleteInstanceCallback = ( err: ServiceError | null, apiResponse?: google.protobuf.Empty @@ -209,9 +209,8 @@ Please use the format 'my-instance' or '${bigtable.projectName}/instances/my-ins return new AppProfile(this, name); } - create(options?: InstanceOptions): Promise; + create(options: InstanceOptions): Promise; create(options: InstanceOptions, callback: CreateInstanceCallback): void; - create(callback: CreateInstanceCallback): void; /** * Create an instance. * @@ -231,14 +230,10 @@ Please use the format 'my-instance' or '${bigtable.projectName}/instances/my-ins * region_tag:bigtable_create_instance */ create( - optionsOrCallback?: InstanceOptions | CreateInstanceCallback, - cb?: CreateInstanceCallback + options: InstanceOptions, + callback?: CreateInstanceCallback ): void | Promise { - const options = - typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; - const callback = - typeof optionsOrCallback === 'function' ? optionsOrCallback : cb!; - this.bigtable.createInstance(this.id, options, callback); + this.bigtable.createInstance(this.id, options, callback!); } createAppProfile( diff --git a/system-test/bigtable.ts b/system-test/bigtable.ts index d82f8647a..c065b0e49 100644 --- a/system-test/bigtable.ts +++ b/system-test/bigtable.ts @@ -83,8 +83,9 @@ describe('Bigtable', () => { describe('instances', () => { it('should get a list of instances', async () => { - const [instances] = await bigtable.getInstances(); + const [instances, failedLocations] = await bigtable.getInstances(); assert(instances.length > 0); + assert(Array.isArray(failedLocations)); }); it('should check if an instance exists', async () => { diff --git a/test/cluster.ts b/test/cluster.ts index 15782f61f..99a913c9c 100644 --- a/test/cluster.ts +++ b/test/cluster.ts @@ -374,65 +374,57 @@ describe('Bigtable/Cluster', () => { cluster.setMetadata({}, done); }); - it('should respect the location option', done => { + it('should respect the nodes option', done => { const options = { - location: 'us-centralb-1', - }; - - const getLocation = Cluster.getLocation_; - const fakeLocation = 'a/b/c/d'; - - Cluster.getLocation_ = (project: string, location: string) => { - assert.strictEqual(project, PROJECT_ID); - assert.strictEqual(location, options.location); - return fakeLocation; + nodes: 3, }; // eslint-disable-next-line @typescript-eslint/no-explicit-any cluster.bigtable.request = (config: any) => { - assert.strictEqual(config.reqOpts.location, fakeLocation); - Cluster.getLocation_ = getLocation; + assert.strictEqual(config.reqOpts.serveNodes, options.nodes); done(); }; cluster.setMetadata(options, assert.ifError); }); - it('should respect the nodes option', done => { + it('should accept and pass user provided input through', done => { const options = { nodes: 3, + location: 'us-west2-b', + defaultStorageType: 'exellent_type', }; + const expectedReqOpts = Object.assign( + {}, + {name: CLUSTER_NAME, serveNodes: options.nodes}, + options + ); + delete expectedReqOpts.nodes; + // eslint-disable-next-line @typescript-eslint/no-explicit-any cluster.bigtable.request = (config: any) => { - assert.strictEqual(config.reqOpts.serveNodes, options.nodes); + assert.deepStrictEqual(config.reqOpts, expectedReqOpts); done(); }; cluster.setMetadata(options, assert.ifError); }); - it('should respect the storage option', done => { + it('should respect the gaxOptions', done => { const options = { - storage: 'ssd', - }; - - const getStorageType = Cluster.getStorageType_; - const fakeStorageType = 'a'; - - Cluster.getStorageType_ = (storage: {}) => { - assert.strictEqual(storage, options.storage); - return fakeStorageType; + nodes: 3, }; + const gaxOptions = {}; // eslint-disable-next-line @typescript-eslint/no-explicit-any cluster.bigtable.request = (config: any) => { - assert.strictEqual(config.reqOpts.defaultStorageType, fakeStorageType); - Cluster.getStorageType_ = getStorageType; + assert.strictEqual(config.reqOpts.serveNodes, options.nodes); + assert.strictEqual(config.gaxOpts, gaxOptions); done(); }; - cluster.setMetadata(options, assert.ifError); + cluster.setMetadata(options, gaxOptions, assert.ifError); }); it('should execute callback with all arguments', done => { diff --git a/test/index.ts b/test/index.ts index dcef0ef8b..8e088eb62 100644 --- a/test/index.ts +++ b/test/index.ts @@ -150,6 +150,11 @@ describe('Bigtable', () => { assert(promisified); }); + it('should work without new', () => { + const bigtable = Bigtable(); + assert(bigtable instanceof Bigtable); + }); + it('should initialize the API object', () => { assert.deepStrictEqual(bigtable.api, {}); }); @@ -337,6 +342,15 @@ describe('Bigtable', () => { describe('createInstance', () => { const INSTANCE_ID = 'my-instance'; + const CLUSTER = { + id: 'my-cluster', + location: 'us-central1-b', + nodes: 3, + storage: 'ssd', + }; + const OPTIONS = { + clusters: [CLUSTER], + }; it('should provide the proper request options', done => { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -349,23 +363,28 @@ describe('Bigtable', () => { assert.strictEqual(config.gaxOpts, undefined); done(); }; - bigtable.createInstance(INSTANCE_ID, assert.ifError); + bigtable.createInstance(INSTANCE_ID, OPTIONS, assert.ifError); }); it('should accept gaxOptions', done => { const gaxOptions = {}; + const options = Object.assign({}, OPTIONS, {gaxOptions}); // eslint-disable-next-line @typescript-eslint/no-explicit-any bigtable.request = (config: any) => { assert.strictEqual(config.gaxOpts, gaxOptions); done(); }; - bigtable.createInstance(INSTANCE_ID, {gaxOptions}, assert.ifError); + bigtable.createInstance(INSTANCE_ID, options, assert.ifError); }); it('should respect the displayName option', done => { - const options = { - displayName: 'robocop', - }; + const options = Object.assign( + {}, + { + displayName: 'robocop', + }, + OPTIONS + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any bigtable.request = (config: any) => { assert.strictEqual( @@ -378,7 +397,7 @@ describe('Bigtable', () => { }); it('should respect the type option', done => { - const options = {type: 'development'}; + const options = Object.assign({}, {type: 'development'}, OPTIONS); const fakeTypeType = 99; // eslint-disable-next-line @typescript-eslint/no-explicit-any FakeInstance.getTypeType_ = (type: string) => { @@ -394,11 +413,15 @@ describe('Bigtable', () => { }); it('should respect the labels option', done => { - const options = { - labels: { - env: 'prod', + const options = Object.assign( + {}, + { + labels: { + env: 'prod', + }, }, - }; + OPTIONS + ); // eslint-disable-next-line @typescript-eslint/no-explicit-any bigtable.request = (config: any) => { assert.deepStrictEqual(config.reqOpts.instance.labels, options.labels); @@ -408,24 +431,15 @@ describe('Bigtable', () => { }); it('should respect the clusters option', done => { - const cluster = { - id: 'my-cluster', - location: 'us-central1-b', - nodes: 3, - storage: 'ssd', - }; - const options = { - clusters: [cluster], - }; const fakeLocation = 'a/b/c/d'; FakeCluster.getLocation_ = (project: string, location: string) => { assert.strictEqual(project, PROJECT_ID); - assert.strictEqual(location, cluster.location); + assert.strictEqual(location, OPTIONS.clusters[0].location); return fakeLocation; }; const fakeStorage = 20; FakeCluster.getStorageType_ = (storage: {}) => { - assert.strictEqual(storage, cluster.storage); + assert.strictEqual(storage, OPTIONS.clusters[0].storage); return fakeStorage; }; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -433,7 +447,7 @@ describe('Bigtable', () => { assert.deepStrictEqual(config.reqOpts.clusters, { 'my-cluster': { location: fakeLocation, - serveNodes: cluster.nodes, + serveNodes: OPTIONS.clusters[0].nodes, defaultStorageType: fakeStorage, }, }); @@ -441,7 +455,7 @@ describe('Bigtable', () => { done(); }; - bigtable.createInstance(INSTANCE_ID, options, assert.ifError); + bigtable.createInstance(INSTANCE_ID, OPTIONS, assert.ifError); }); it('should return an error to the callback', done => { @@ -451,6 +465,7 @@ describe('Bigtable', () => { }; bigtable.createInstance( INSTANCE_ID, + OPTIONS, (err: Error, instance: Instance, operation: gax.Operation) => { assert.strictEqual(err, error); assert.strictEqual(instance, undefined); @@ -476,6 +491,7 @@ describe('Bigtable', () => { }; bigtable.createInstance( INSTANCE_ID, + OPTIONS, (err: Error, instance: Instance, ...args: Array<{}>) => { assert.ifError(err); assert.strictEqual(instance, fakeInstance); @@ -503,6 +519,18 @@ describe('Bigtable', () => { bigtable.createInstance(INSTANCE_ID, options); }, /A cluster was provided without an `id` property defined\./); }); + + it('should throw an error if configuration object is not provided', () => { + assert.throws(() => { + bigtable.createInstance(INSTANCE_ID, assert.ifError); + }, /A configuration object is required to create an instance\./); + }); + + it('should throw an error if cluster configuration is not provided', () => { + assert.throws(() => { + bigtable.createInstance(INSTANCE_ID, {}, assert.ifError); + }, /At least one cluster configuration object is required to create an instance\./); + }); }); describe('getInstances', () => { @@ -541,7 +569,7 @@ describe('Bigtable', () => { }); }); - it('should return an array of instance objects', done => { + it('should return an array of instance objects and failed locations', done => { const response = { instances: [ { @@ -551,6 +579,7 @@ describe('Bigtable', () => { name: 'b', }, ], + failedLocations: ['projects//locations/'], }; const fakeInstances = [{}, {}]; bigtable.request = (config: {}, callback: Function) => { @@ -563,12 +592,18 @@ describe('Bigtable', () => { }; bigtable.getInstances( - (err: Error, instances: Instance[], apiResponse: {}) => { + ( + err: Error, + instances: Instance[], + failedLocations: string[], + apiResponse: {} + ) => { assert.ifError(err); assert.strictEqual(instances[0], fakeInstances[0]); assert.strictEqual(instances[0].metadata, response.instances[0]); assert.strictEqual(instances[1], fakeInstances[1]); assert.strictEqual(instances[1].metadata, response.instances[1]); + assert.strictEqual(failedLocations, response.failedLocations); assert.strictEqual(apiResponse, response); done(); } diff --git a/test/instance.ts b/test/instance.ts index 73b2b7006..1bbb4ebeb 100644 --- a/test/instance.ts +++ b/test/instance.ts @@ -95,7 +95,10 @@ class FakeTable extends Table { describe('Bigtable/Instance', () => { const INSTANCE_ID = 'my-instance'; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const BIGTABLE = {projectName: 'projects/my-project'} as Bigtable; + const BIGTABLE = { + projectName: 'projects/my-project', + projectId: 'my-project', + } as Bigtable; const INSTANCE_NAME = `${BIGTABLE.projectName}/instances/${INSTANCE_ID}`; const APP_PROFILE_ID = 'my-app-profile'; const CLUSTER_ID = 'my-cluster'; @@ -201,18 +204,6 @@ describe('Bigtable/Instance', () => { }; instance.create(options, done); }); - - it('should not require options', done => { - (instance.bigtable.createInstance as Function) = ( - id: string, - options: {}, - callback: Function - ) => { - assert.deepStrictEqual(options, {}); - callback(); // done() - }; - instance.create(done); - }); }); describe('createAppProfile', () => { @@ -404,6 +395,7 @@ describe('Bigtable/Instance', () => { it('should respect the nodes option', done => { const options = { nodes: 3, + location: 'us-central1-c', }; // eslint-disable-next-line @typescript-eslint/no-explicit-any (instance.bigtable.request as Function) = (config: any) => { @@ -1108,6 +1100,16 @@ describe('Bigtable/Instance', () => { instance.setMetadata(metadata, done); }); + it('should accept gaxOptions', done => { + const gaxOptions = {}; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (instance.bigtable.request as Function) = (config: any) => { + assert.strictEqual(config.gaxOpts, gaxOptions); + done(); + }; + instance.setMetadata({}, gaxOptions, assert.ifError); + }); + it('should update metadata property with API response', done => { const response = {}; sandbox