samples: Bigtable: Added App Profile samples and tests #1456
Conversation
google-cla
bot
added
the
cla: yes
product-auto-label
bot
added
the
samples
|
Here is the summary of changes. You are about to add 5 region tags.
This comment is generated by snippet-bot.
|
bshaffer
left a comment
bshaffer
left a comment
There was a problem hiding this comment.
The main issues I see in this PR are mostly nits, but please check for:
- Using double quotes when you should have single quotes
- Lack of implicit else
- spaces needed around
=> - in
try/catchmake sure the special cases return instead of executing the rest of the sample.
Changed double quotes to single quotes Removed redundant else blocks Converted echo statements to printf
| // $appProfile->setMultiClusterRoutingUseAny($routingPolicy); | ||
|
|
||
| // returns a string identifier depending on SingleClusterRouting or MultiClusterRoutingUseAny | ||
| $routingPolicyStr = $appProfile->getRoutingPolicy(); |
There was a problem hiding this comment.
This returns single_cluster_routing or multi_cluster_routing_any. But TBH, it feels like the field that the user should update is routing_policy.
There was a problem hiding this comment.
If you take a look at the API Reference, you'll see there are two fields as part of the AppProfile object being updated: single_cluster_routing and multi_cluster_routing_any. routing_policy, even though it's a protobuf getter, is not the field to be updated, it's only the field identifier. So this code is correct.
* Bigtable: Added App Profile samples and tests * Bigtable: Addressed PR comments for App Profile samples Changed double quotes to single quotes Removed redundant else blocks Converted echo statements to printf * Changed quotes for bigtableTest * Fixed lint errors Co-authored-by: Brent Shaffer <[email protected]>
2 participants
The tests should pass once #1450 is merged.