Skip to content

Commit

Permalink
feat: Remove need to pass location parameter along (#1093)
Browse files Browse the repository at this point in the history
* snapshot tests for create cluster

* change request to config

* More snapshot tests

* Refactor constants out

* Add generated testcases for partial cluster update

* Add header to constants file

* Remove need to pass location along

* Remove comment

* eliminate compile error

* lint fix
  • Loading branch information
danieljbruce authored Jun 20, 2022
1 parent 886af7a commit 75c1a30
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 42 deletions.
7 changes: 2 additions & 5 deletions src/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export interface SetClusterMetadataOptions {
minServeNodes?: number;
maxServeNodes?: number;
cpuUtilizationPercent?: number;
location?: string;
}
export type SetClusterMetadataCallback = GenericOperationCallback<
Operation | null | undefined
Expand Down Expand Up @@ -704,11 +705,7 @@ Please use the format 'my-cluster' or '${instance.name}/clusters/my-cluster'.`);
typeof gaxOptionsOrCallback === 'object'
? gaxOptionsOrCallback
: ({} as CallOptions);
const reqOpts = ClusterUtils.getRequestFromMetadata(
metadata,
this?.metadata?.location,
this.name
);
const reqOpts = ClusterUtils.getRequestFromMetadata(metadata, this.name);
this.bigtable.request<Operation>(
{
client: 'BigtableInstanceAdminClient',
Expand Down
10 changes: 8 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,9 +612,15 @@ export class Bigtable {
);
}
ClusterUtils.validateClusterMetadata(cluster);
const clusterClone = Object.assign({}, cluster);
if (clusterClone.location) {
clusterClone.location = Cluster.getLocation_(
this.projectId,
clusterClone.location
);
}
clusters[cluster.id!] = ClusterUtils.getClusterBaseConfig(
cluster,
Cluster.getLocation_(this.projectId, cluster.location!),
clusterClone,
undefined
);
Object.assign(clusters[cluster.id!], {
Expand Down
12 changes: 8 additions & 4 deletions src/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,11 +394,15 @@ Please use the format 'my-instance' or '${bigtable.projectName}/instances/my-ins
} as google.bigtable.admin.v2.CreateClusterRequest;
ClusterUtils.validateClusterMetadata(options);
if (!is.empty(options)) {
const optionsClone = Object.assign({}, options);
if (optionsClone.location) {
optionsClone.location = Cluster.getLocation_(
this.bigtable.projectId,
optionsClone.location
);
}
reqOpts.cluster = ClusterUtils.getClusterBaseConfig(
options,
options.location
? Cluster.getLocation_(this.bigtable.projectId, options.location)
: undefined,
optionsClone,
undefined
);
}
Expand Down
20 changes: 6 additions & 14 deletions src/utils/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@
// limitations under the License.

import * as protos from '../../protos/protos';
import {
BasicClusterConfig,
ICluster,
SetClusterMetadataOptions,
} from '../cluster';
import {ICluster, SetClusterMetadataOptions} from '../cluster';
import {google} from '../../protos/protos';

export class ClusterUtils {
Expand All @@ -28,9 +24,7 @@ export class ClusterUtils {
static incompleteConfigError =
'All of autoscaling configurations must be specified at the same time (min_serve_nodes, max_serve_nodes, and cpu_utilization_percent).';

static validateClusterMetadata(
metadata: SetClusterMetadataOptions | BasicClusterConfig
): void {
static validateClusterMetadata(metadata: SetClusterMetadataOptions): void {
if (metadata.nodes) {
if (
metadata.minServeNodes ||
Expand Down Expand Up @@ -92,8 +86,7 @@ export class ClusterUtils {
}

static getClusterBaseConfig(
metadata: SetClusterMetadataOptions | BasicClusterConfig,
location: string | undefined | null,
metadata: SetClusterMetadataOptions,
name: string | undefined
): google.bigtable.admin.v2.ICluster {
let clusterConfig;
Expand All @@ -114,6 +107,7 @@ export class ClusterUtils {
},
};
}
const location = metadata?.location;
return Object.assign(
{},
name ? {name} : null,
Expand All @@ -125,12 +119,11 @@ export class ClusterUtils {

static getClusterFromMetadata(
metadata: SetClusterMetadataOptions,
location: string | undefined | null,
name: string
): google.bigtable.admin.v2.ICluster {
const cluster: ICluster | SetClusterMetadataOptions = Object.assign(
{},
this.getClusterBaseConfig(metadata, location, name),
this.getClusterBaseConfig(metadata, name),
metadata
);
delete (cluster as SetClusterMetadataOptions).nodes;
Expand All @@ -142,11 +135,10 @@ export class ClusterUtils {

static getRequestFromMetadata(
metadata: SetClusterMetadataOptions,
location: string | undefined | null,
name: string
): protos.google.bigtable.admin.v2.IPartialUpdateClusterRequest {
return {
cluster: this.getClusterFromMetadata(metadata, location, name),
cluster: this.getClusterFromMetadata(metadata, name),
updateMask: {paths: this.getUpdateMask(metadata)},
};
}
Expand Down
1 change: 0 additions & 1 deletion test/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,6 @@ describe('Bigtable/Cluster', () => {

const expectedReqOpts = ClusterUtils.getRequestFromMetadata(
options,
options.location,
CLUSTER_NAME
);

Expand Down
24 changes: 8 additions & 16 deletions test/utils/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,17 @@ describe('Bigtable/Utils/Cluster', () => {
it('should translate metadata for a full set of parameters', () => {
const metadata = {
nodes: 1,
location: 'projects/{{projectId}}/locations/us-east4-b',
minServeNodes: 2,
maxServeNodes: 3,
cpuUtilizationPercent: 50,
};
const name = 'cluster1';
const location = 'projects/{{projectId}}/locations/us-east4-b';
const reqOpts = ClusterUtils.getRequestFromMetadata(
metadata,
location,
name
);
const reqOpts = ClusterUtils.getRequestFromMetadata(metadata, name);
assert.deepStrictEqual(reqOpts, {
cluster: {
name: name,
location: location,
name,
location: metadata.location,
serveNodes: metadata.nodes,
clusterConfig: {
clusterAutoscalingConfig: {
Expand All @@ -61,21 +57,17 @@ describe('Bigtable/Utils/Cluster', () => {
});
it('should translate metadata for autoscaling parameters', () => {
const metadata = {
location: 'projects/{{projectId}}/locations/us-east4-b',
minServeNodes: 2,
maxServeNodes: 3,
cpuUtilizationPercent: 50,
};
const name = 'cluster1';
const location = 'projects/{{projectId}}/locations/us-east4-b';
const reqOpts = ClusterUtils.getRequestFromMetadata(
metadata,
location,
name
);
const reqOpts = ClusterUtils.getRequestFromMetadata(metadata, name);
assert.deepStrictEqual(reqOpts, {
cluster: {
name: name,
location: location,
name,
location: metadata.location,
clusterConfig: {
clusterAutoscalingConfig: {
autoscalingLimits: {
Expand Down

0 comments on commit 75c1a30

Please sign in to comment.