docs: Update all the samples targeting the BigtableTableAdminClient to use the generated layer instead#1710
Conversation
This commit updates the `backups.restore.js` sample to use the `BigtableTableAdminClient` directly for restoring a table from a backup, instead of going through the handwritten `bigtable.instance().createTableFromBackup()` method. This change is based on the design document for the Node.js Bigtable Admin API autogeneration. The corresponding test for the sample in `samples/test/backups.js` has also been updated to pass the correct arguments to the updated sample script.
instance of the admin client should be used
gkevinzheng
left a comment
There was a problem hiding this comment.
Could we add a code sample for how to generate a polling harness for GenerateConsistencyToken -> CheckConsistency polling?
| await operation.promise(); | ||
| console.log(`Table restored to ${table.id} successfully.`); | ||
| const [table] = await operation.promise(); | ||
|
|
There was a problem hiding this comment.
Do we want to document how to access the OptimizeRestoredTable LRO here?
There was a problem hiding this comment.
I added a comment here for that.
There was a problem hiding this comment.
I'm not sure a comment here covers accessing the OptimizeRestoredTable LRO after the RestoreTable LRO finishes. In Python, we had to read the RestoreTableMetadata, use the GAPIC operation client to get the LRO pointed to by the optimize_table_operation_name field in the LRO metadata, then return that to the user so they could await that.
I think your sample should either encompass all of that or expose a field/function to the user that abstracts all of that away.
There was a problem hiding this comment.
I agree. Could you add a section that awaits the optimized table token and uses that to restore the table?
There was a problem hiding this comment.
I'll add a line for adding the optimized table.
|
Here is the summary of changes. You are about to add 2 region tags.
This comment is generated by snippet-bot.
|
I added a commit here for that. |
…into fix-restore-table-sample-3 # Conflicts: # samples/api-reference-doc-snippets/backups.restore.js
| name: optimizeTableOperationName, | ||
| }); | ||
| } | ||
| console.log( |
There was a problem hiding this comment.
You need to also await the OptimizeRestoreTable LRO in addition to awaiting getting the operation.
There was a problem hiding this comment.
The while loop intends to wait until the optimize restore table operation is complete. I'm not sure if Node has a method out of the box that lets us get an LRO for optimize restore table so we do the equivalent which involves getting the operation until operation.done is true.
Description
In this pull request we migrate the rest of the samples that use the handwritten layer which calls BigtableTableAdminClient under the hood to instead call the BigtableTableAdminClient directly. This is a follow-up PR to #1697.
Impact
This is a crucial step towards deprecating handwritten methods that use the admin client because it encourages users to start migrating their code over to using the admin clients directly.
Testing
This is a change to the samples so
npm run samples-testis already a check in place that ensures these are correct.We should consider extracting each sample into its own file and running each file just to ensure it does its job.