chore: migrate Spanner core samples batch 4 (exact copy)#4279
chore: migrate Spanner core samples batch 4 (exact copy)#4279angelcaamal wants to merge 1 commit intoGoogleCloudPlatform:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of new Cloud Spanner code samples covering database management (default leader, DDL), instance configurations (CRUD, operations), instance partitions, sequences, and observability with OpenTelemetry. Additionally, it enables several system tests that were previously skipped. Feedback includes correcting a bug in instance-config-update.js where a global proto reference was being overwritten, improving safety when handling optional replicas in instance-config-create.js, and fixing a default parameter typo in observability-traces.js.
| updateMask: (protos.google.protobuf.FieldMask = { | ||
| paths: ['display_name', 'labels'], | ||
| }), |
There was a problem hiding this comment.
The assignment to protos.google.protobuf.FieldMask inside the object literal is a bug. It overwrites the FieldMask reference in the protos object, which could lead to unexpected behavior in other parts of the application. It should simply be an object literal.
| updateMask: (protos.google.protobuf.FieldMask = { | |
| paths: ['display_name', 'labels'], | |
| }), | |
| updateMask: { | |
| paths: ['display_name', 'labels'], | |
| }, |
| replicas: baseInstanceConfig.replicas.concat( | ||
| baseInstanceConfig.optionalReplicas[0] | ||
| ), |
There was a problem hiding this comment.
Accessing baseInstanceConfig.optionalReplicas[0] without checking if the array exists or has elements can lead to undefined being passed to concat. This results in an array containing undefined, which will likely cause the API request to fail. Using slice(0, 1) is a safer way to get at most one element as an array.
| replicas: baseInstanceConfig.replicas.concat( | |
| baseInstanceConfig.optionalReplicas[0] | |
| ), | |
| replicas: baseInstanceConfig.replicas.concat( | |
| (baseInstanceConfig.optionalReplicas || []).slice(0, 1) | |
| ), |
| async function main( | ||
| projectId = 'my-project-id', | ||
| instanceId = 'my-instance-id', | ||
| databaseId = 'my-project-id' |
Description
Fixes internal: b/500480522
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
npm test(see Testing)npm run lint(see Style)GoogleCloudPlatform/nodejs-docs-samples. Not a fork.