feat: Remove need to pass location parameter along #1093
Conversation
product-auto-label
bot
added
size: l
danieljbruce
added
the
do not merge
src/cluster.ts
Outdated
| @@ -693,7 +693,7 @@ Please use the format 'my-cluster' or '${instance.name}/clusters/my-cluster'.`); | |||
| * region_tag:bigtable_api_cluster_set_meta | |||
| */ | |||
| setMetadata( | |||
| metadata: SetClusterMetadataOptions, | |||
| metadata: BasicClusterConfig, | |||
There was a problem hiding this comment.
This is not a breaking change because BasicClusterConfig includes all the parameters that SetClusterMetadataOptions includes. We need to do this to allow the user to pass in location parameter, which shouldn't be required, but seems to be accepted in other codebases when we update clusters.
There was a problem hiding this comment.
I believe this will be a compilation error for upstream users because location is require3d on BasicClusterConfig, what I might do instead is add location as an optional parameter on SetClusterMetadataOptions.
There was a problem hiding this comment.
Great point. By adding this optional parameter to the interface it also means we make fewer changes too which is good because it keeps this PR small. Done.
src/cluster.ts
Outdated
| @@ -693,7 +693,7 @@ Please use the format 'my-cluster' or '${instance.name}/clusters/my-cluster'.`); | |||
| * region_tag:bigtable_api_cluster_set_meta | |||
| */ | |||
| setMetadata( | |||
| metadata: SetClusterMetadataOptions, | |||
| metadata: BasicClusterConfig, | |||
There was a problem hiding this comment.
I believe this will be a compilation error for upstream users because location is require3d on BasicClusterConfig, what I might do instead is add location as an optional parameter on SetClusterMetadataOptions.
…into autoscaler-refactor-tech-debt
product-auto-label
bot
added
size: m
src/cluster.ts
Outdated
| @@ -693,7 +693,7 @@ Please use the format 'my-cluster' or '${instance.name}/clusters/my-cluster'.`); | |||
| * region_tag:bigtable_api_cluster_set_meta | |||
| */ | |||
| setMetadata( | |||
| metadata: SetClusterMetadataOptions, | |||
| metadata: BasicClusterConfig, | |||
There was a problem hiding this comment.
Great point. By adding this optional parameter to the interface it also means we make fewer changes too which is good because it keeps this PR small. Done.
danieljbruce
added
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
danieljbruce
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
This PR is the 2nd step towards refactoring code pertaining to autoscaler and cluster update work. This should be merged after the following PR:
#1092