fix(types)!: improve types in index.ts (#720)

BREAKING CHANGE: cluster.setMetadata(): only node count is updatable on an existing cluster; getInstancesCallback/Response: dropped nextQuery property as it is deprecated for this method, exposed failedLocations property; instance.createCluster(): removed unsupported params serveNodes and defaultStorageType

Author: AVaksman

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<SetClusterMetadataResponse>;
   setMetadata(
-    metadata: CreateClusterOptions,
-    gaxOptions: CallOptions
-  ): Promise<SetClusterMetadataResponse>;
-  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<SetClusterMetadataResponse> {
     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<Operation>(
       {

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<CreateInstanceResponse>;
   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.<string, string>} [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<CreateInstanceResponse> {
-    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 <caption>To control how many API requests are made and page
-   * through the results manually, set `autoPaginate` to `false`.</caption>
-   * function callback(err, instances, nextQuery, apiResponse) {
-   *   if (nextQuery) {
-   *     // More results exist.
-   *     bigtable.getInstances(nextQuery, callback);
-   *   }
-   * }
-   *
-   * bigtable.getInstances({
-   *   autoPaginate: false
-   * }, callback);
-   *
    * @example <caption>If the callback is omitted, we'll return a Promise.
    * </caption>
    * 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);
       }
     );
   }

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<Resource> {
+  (
+    err: ServiceError | null,
+    resource?: Resource,
+    operation?: Operation,
+    apiResponse?: IOperation
+  ): void;
+}
+export type CreateInstanceCallback = LongRunningResourceCallback<Instance>;
+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<CreateInstanceResponse>;
+  create(options: InstanceOptions): Promise<CreateInstanceResponse>;
   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<CreateInstanceResponse> {
-    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(

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 () => {