Skip to content

Commit

Permalink
fix(types)!: improve types in index.ts (#720)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
AVaksman committed Jun 15, 2020
1 parent 21f8fef commit 508d1f9
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 154 deletions.
56 changes: 25 additions & 31 deletions src/cluster.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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>(
{
Expand Down
64 changes: 32 additions & 32 deletions src/index.ts
Expand Up @@ -27,7 +27,6 @@ import {
InstanceOptions,
CreateInstanceCallback,
CreateInstanceResponse,
ClusterInfo,
IInstance,
} from './instance';
import {shouldRetryRequest} from './decorateStatus';
Expand All @@ -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
];

Expand Down Expand Up @@ -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.
*
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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.'
Expand All @@ -581,7 +584,7 @@ export class Bigtable {
};

return clusters;
}, {} as {[index: string]: ClusterInfo});
}, {} as {[index: string]: google.bigtable.admin.v2.ICluster});

this.request(
{
Expand All @@ -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.
*/
/**
Expand All @@ -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(
Expand Down Expand Up @@ -683,7 +683,7 @@ export class Bigtable {
instance.metadata = instanceData;
return instance;
});
callback!(null, instances, resp);
callback!(null, instances, resp.failedLocations, resp);
}
);
}
Expand Down
43 changes: 19 additions & 24 deletions src/instance.ts
Expand Up @@ -32,6 +32,8 @@ import {
CreateClusterResponse,
GetClustersCallback,
GetClustersResponse,
IOperation,
BasicClusterConfig,
} from './cluster';
import {Family} from './family';
import {
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
*
Expand All @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion system-test/bigtable.ts
Expand Up @@ -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 () => {
Expand Down

0 comments on commit 508d1f9

Please sign in to comment.