-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: add storage control anywhere cache samples #4177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add storage control anywhere cache samples #4177
Conversation
Here is the summary of changes. You are about to add 7 region tags.
This comment is generated by snippet-bot.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mostly looks ok. I have a few questions. I did run this through our generation to see what kind of output I received and I've shared those headers for visibility. I think there are a few areas of improvements to make sure there is a try..catch to ensure folks get actionable feedback so they don't assume the code is broken depending on the order in which they run commands.
Thanks!
Thank you for the suggestion. Our samples are designed to demonstrate the simplest "happy path" for a feature, focusing on the core API call. We intentionally omit try...catch blocks to keep the code concise and readable, which helps maintain a consistent style across all of our language samples. Furthermore, unhandled rejections are already handled at the application level to prevent the code from breaking, so the primary goal of our samples remains focused on positive-case demonstration. |
e83ba77
to
b5e4d63
Compare
b5e4d63
to
a15ae91
Compare
@iennae Just a friendly reminder to review this PR when you get a chance. Thanks! |
a15ae91
to
bb23814
Compare
@iennae That's great feedback, thank you for running the samples and sharing the output headers—having that visibility is really helpful. The issues have been fixed and pushed here; could you please take a look? |
Improve Anywhere Cache API samples (Documentation & Error Handling) * Wraps all asynchronous Anywhere Cache API samples (Create, Get, List, Disable, Pause, Resume) in `try...catch` blocks for production readiness. * Adds specific gRPC error code checks (NOT_FOUND, FAILED_PRECONDITION) to provide better diagnostic feedback to users. * Clarifies documentation regarding the optional nature and default values of `ttl` and `admissionPolicy` for cache creation.
fd4b63b
to
572f7f1
Compare
…rage Updates tests for all Anywhere Cache management methods (Create, Get, List, Resume, Disable, Pause) to align with enhanced sample script output. **(Tests were skipped due to reliance on Long-Running Operations (LROs) in the sample code.)** This includes: * **Enhanced Assertions:** Asserting against all newly added detail fields (e.g., Name, State, TTL) to verify full API response parsing in Get and List samples. * **Negative Scenario Coverage:** Adding explicit **negative scenario tests** for state-dependent operations (Disable, Pause, Resume) to assert graceful failure on expected errors like `FAILED_PRECONDITION`.
Description
Fixes #
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.