feat(google/datastore/v1): onboard a new library#8856
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the auto-generated @google-cloud/datastore-api package, onboarding client libraries, protobuf definitions, samples, and tests for both the main Datastore and Datastore Admin APIs. However, a critical issue was identified where sequential generation runs caused the admin API generation to overwrite several shared files. As a result, DatastoreClient is completely missing from the package exports, which breaks the test suite. Additionally, the package description, README links, and webpack configuration are incorrect because they only reference the admin client instead of the entire Datastore API. The generation orchestrator or configuration needs to be updated to ensure both clients are generated and exported together.
| // ** https://github.com/googleapis/gapic-generator-typescript ** | ||
| // ** All changes to this file may be overwritten. ** | ||
|
|
||
| export { DatastoreAdminClient } from './datastore_admin_client'; |
There was a problem hiding this comment.
The 'DatastoreClient' (from 'google/datastore/v1') is generated in 'src/v1/datastore_client.ts' but is completely missing from the exports in 'src/v1/index.ts'. This typically happens when the generator is run sequentially for multiple APIs, causing the second run (for 'admin/v1') to overwrite the index files generated by the first run. Since these are auto-generated files, please do not edit them manually. Instead, the generation orchestrator or configuration should be updated to ensure both clients are generated and exported together without overwriting each other.
References
- Do not manually edit generated TypeScript client files. Any changes or updates must be made upstream in the generator.
| export {v1, DatastoreAdminClient}; | ||
| export default {v1, DatastoreAdminClient}; |
There was a problem hiding this comment.
Only 'DatastoreAdminClient' is exported here. 'DatastoreClient' is completely omitted from the package's main entry point. This is a critical issue resulting from the sequential generation overwrite of the index files. Please update the generation process to ensure both clients are correctly exported from the package.
References
- Do not manually edit generated TypeScript client files. Any changes or updates must be made upstream in the generator.
|
|
||
| it('should create a client with no option', () => { | ||
| const client = new datastoreModule.v1.DatastoreClient(); | ||
| assert(client); |
There was a problem hiding this comment.
Because 'DatastoreClient' is not exported in 'src/v1/index.ts', importing and instantiating 'datastoreModule.v1.DatastoreClient' here will fail at compile time and runtime. This confirms that the sequential generation overwrite of the index files has broken the build and test suite for the main 'DatastoreClient'.
References
- Do not manually edit generated TypeScript client files. Any changes or updates must be made upstream in the generator.
| { | ||
| "name": "@google-cloud/datastore-api", | ||
| "version": "0.1.0", | ||
| "description": "Admin client for Node.js", |
There was a problem hiding this comment.
The description 'Admin client for Node.js' is misleading for a package that is supposed to onboard the entire Datastore API (including the main 'DatastoreClient'). This is another symptom of the sequential generation overwrite issue, where the second generation run (for 'admin/v1') overwrote the 'package.json' from the first run. Please ensure the generation process merges or correctly sets the package metadata.
References
- Do not manually edit generated client configuration JSON files. Any changes or updates must be made upstream in the generator.
| [homepage_samples]: https://github.com/googleapis/google-cloud-node/blob/main/packages/google-datastore-admin/samples | ||
| [homepage_changelog]: https://github.com/googleapis/google-cloud-node/blob/main/packages/google-datastore-admin/CHANGELOG.md | ||
| [homepage]: https://github.com/googleapis/google-cloud-node/blob/main/packages/google-datastore-admin |
There was a problem hiding this comment.
The generated links at the bottom of the README point to 'packages/google-datastore-admin' instead of 'packages/google-cloud-datastore-api'. This is likely because the generator used the last API path ('google/datastore/admin/v1') to construct these links during the sequential generation. Please ensure the generation orchestrator correctly uses the package directory name 'google-cloud-datastore-api' for these links.
References
- Do not manually edit auto-generated files to fix typos or make other changes, as these edits will be overwritten during the next regeneration. Instead, apply the fixes upstream in the generator or templates.
| library: 'DatastoreAdmin', | ||
| filename: './datastore-admin.js', |
There was a problem hiding this comment.
The webpack configuration exports the library as 'DatastoreAdmin' and outputs to 'datastore-admin.js'. For a unified 'google-cloud-datastore-api' package, this should ideally represent the entire Datastore API (or both clients). This is another side-effect of the sequential generation overwrite where the admin client's configuration overwrote the main client's configuration.
quirogas
changed the title
quirogas
force-pushed
the
feat/onboard-datastore-api
branch
from
c270c15 to
d65d7a6
Compare
1 participant
No description provided.