Move the samples over#8848
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of Node.js samples for Cloud Spanner, including archived versions, along with configuration files like .eslintrc.yml, package.json, and an auto-generated README.md. The review comments highlight several critical issues: incorrect handling of the exists() promise return value in backups-delete.js and instance-config-delete.js (which returns an array rather than a direct boolean), broken relative require paths in datatypes.js, and misplaced OpenTelemetry dependencies in package.json that should be under dependencies instead of devDependencies. Additionally, minor typos and a duplicate entry were identified in the README.md file.
| await backup.delete(); | ||
|
|
||
| // Verify backup no longer exists | ||
| const exists = await backup.exists(); |
There was a problem hiding this comment.
| const {updateWithNumericData} = require('../numeric-update-data'); | ||
| const {queryWithNumericParameter} = require('../numeric-query-parameter'); | ||
|
|
||
| const {addJsonColumn} = require('./json-add-column'); | ||
| const {updateWithJsonData} = require('../json-update-data'); | ||
| const {queryWithJsonParameter} = require('../json-query-parameter'); |
There was a problem hiding this comment.
The require paths for the numeric and json samples are incorrect. They are pointing to the parent directory (../) instead of the current directory (./). This will cause the sample to fail when run.
| const {updateWithNumericData} = require('../numeric-update-data'); | |
| const {queryWithNumericParameter} = require('../numeric-query-parameter'); | |
| const {addJsonColumn} = require('./json-add-column'); | |
| const {updateWithJsonData} = require('../json-update-data'); | |
| const {queryWithJsonParameter} = require('../json-query-parameter'); | |
| const {updateWithNumericData} = require('./numeric-update-data'); | |
| const {queryWithNumericParameter} = require('./numeric-query-parameter'); | |
| const {addJsonColumn} = require('./json-add-column'); | |
| const {updateWithJsonData} = require('./json-update-data'); | |
| const {queryWithJsonParameter} = require('./json-query-parameter'); |
| console.log(`Deleting ${instanceConfig.id}...\n`); | ||
| await instanceConfig.delete(); | ||
| // Verify that the instance config no longer exists | ||
| const exists = await instanceConfig.exists(); |
There was a problem hiding this comment.
| "devDependencies": { | ||
| "@google-cloud/opentelemetry-cloud-trace-exporter": "^2.4.1", | ||
| "@opentelemetry/exporter-trace-otlp-grpc": "^0.57.0", | ||
| "@opentelemetry/instrumentation": "^0.57.0", | ||
| "@opentelemetry/instrumentation-grpc": "^0.57.0", | ||
| "@opentelemetry/resources": "1.30.1", | ||
| "@opentelemetry/sdk-trace-base": "~1.30.1", | ||
| "@opentelemetry/sdk-trace-node": "^1.30.1", | ||
| "@opentelemetry/api": "^1.9.0", | ||
| "@opentelemetry/core": "^1.30.1", |
There was a problem hiding this comment.
The OpenTelemetry packages are used directly in the observability samples (observability-traces.js, observability-traces-otlp.js), so they should be listed under dependencies instead of devDependencies. This ensures they are installed for users who want to run these samples from the published package.
| * [Add and drop new database role](#add-and-drop-new-database-role) | ||
| * [Backups-cancel](#backups-cancel) | ||
| * [Copies a source backup](#copies-a-source-backup) | ||
| * [Copies a source backup](#copies-a-source-backup) |
| __Usage:__ | ||
|
|
||
|
|
||
| `node ppg-numeric-data-type.js <INSTANCE_ID> <DATABASE_ID> <PROJECT_ID>` |
There was a problem hiding this comment.
| __Usage:__ | ||
|
|
||
|
|
||
| `node table-create-with-foreign-key-delete-cascade.js.js <INSTANCE_ID> <DATABASE_ID> <PROJECT_ID>` |
There was a problem hiding this comment.
CI experiment
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕