-
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.
|
/** | ||
* TODO(developer): Uncomment these variables before running the sample. | ||
*/ |
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.
suggestion:
/**
- Creates an Anywhere Cache instance for a Cloud Storage bucket.
- Anywhere Cache is a feature that provides an SSD-backed zonal read cache.
- This can significantly improve read performance for frequently accessed data
- by caching it in the same zone as your compute resources.
- @param {string} bucketName The name of the bucket to create the cache for.
- Example: 'your-gcp-bucket-name'
- @param {string} zone The zone where the cache will be created.
- Example: 'us-central1-a'
*/
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.
Thank you for your thoughtful suggestion and for providing such a well-crafted JSDoc comment. We aim to maintain a consistent documentation style across all our Cloud Storage samples to offer a unified experience for our users.
Adopting a new format, even if it is an improvement, could impact the overall cohesiveness and clarity of the sample set.
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.
Is there a standard currently being followed that we should adhere to for future reference?
const request = { | ||
parent: bucketPath, | ||
anywhereCache: { | ||
zone: zoneName, |
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.
should the default sample show a TTL and/or admissionPolicy ? (reading documentation here to better understand) https://cloud.google.com/storage/docs/anywhere-cache
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.
Thank you for the detailed question. You're right that ttlSeconds
and admissionPolicy
are available fields.
Our sample's goal is to show the simplest, most fundamental usage with only the required parameters. This approach ensures consistency and clarity across all language samples, making it easier for users to get started. While including optional fields would be a good demonstration, we keep the default samples minimal to focus on the core functionality. We appreciate you digging into the documentation!
/** | ||
* TODO(developer): Uncomment these variables before running the sample. | ||
*/ |
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.
suggestion:
/**
- Disables an Anywhere Cache instance.
- Disabling a cache is the first step to permanently removing it. Once disabled,
- the cache stops ingesting new data. After a grace period, the cache and its
- contents are deleted. This is useful for decommissioning caches that are no
- longer needed.
- @param {string} bucketName The name of the bucket where the cache resides.
- Example: 'your-gcp-bucket-name'
- @param {string} cacheName The unique identifier of the cache instance to disable.
- Example: 'cacheName'
*/
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.
Thank you for your thoughtful suggestion and for providing such a well-crafted JSDoc comment. We aim to maintain a consistent documentation style across all our Cloud Storage samples to offer a unified experience for our users.
Adopting a new format, even if it is an improvement, could impact the overall cohesiveness and clarity of the sample set.
|
||
// Run request | ||
const [response] = await controlClient.disableAnywhereCache(request); | ||
console.log(`Disabled anywhere cache: ${response.name}.`); |
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.
nit: based on if this actually disables or triggers it to be disabled:
wrap everything in a try .. catch and log instead:
console.log(Successfully initiated disablement for Anywhere Cache '${cacheName}'.
);
console.log( Current State: ${response.state}
);
catch potential errors
catch (err) {
console.error(Error disabling Anywhere Cache '${cacheName}': ${err.message}
);
if (err.code === 5) {
// NOT_FOUND error can occur if the bucket or cache does not exist.
console.error(Please ensure the cache '${cacheName}' exists in bucket '${bucketName}'.
);
} else if (err.code === 9) {
// FAILED_PRECONDITION can occur if the cache is already being disabled or is not in a RUNNING state.
console.error(Cache '${cacheName}' may not be in a state that allows disabling.
);
}
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.
However, our code samples are designed to demonstrate the simplest, "happy path" usage of a feature. We intentionally omit error handling to keep the code concise and focused on the core API call. This approach ensures all Cloud Storage samples share a consistent, minimal style, making them easier to understand at a glance.
|
||
function main(bucketName, cacheName) { | ||
// [START storage_control_get_anywhere_cache] | ||
/** |
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.
nit: suggestion of
/**
- Retrieves details of a specific Anywhere Cache instance.
- This function is useful for checking the current state, configuration (like TTL),
- and other metadata of an existing cache.
- @param {string} bucketName The name of the bucket where the cache resides.
- Example: 'your-gcp-bucket-name'
- @param {string} cacheName The unique identifier of the cache instance.
- Example: 'my-anywhere-cache-id'
*/
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.
Thank you for your thoughtful suggestion and for providing such a well-crafted JSDoc comment. We aim to maintain a consistent documentation style across all our Cloud Storage samples to offer a unified experience for our users.
Adopting a new format, even if it is an improvement, could impact the overall cohesiveness and clarity of the sample set.
|
||
function main(bucketName) { | ||
// [START storage_control_list_anywhere_caches] | ||
/** |
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.
nit: /**
- Lists all Anywhere Cache instances for a Cloud Storage bucket.
- This function helps you discover all active and pending caches associated with
- a specific bucket, which is useful for auditing and management.
- @param {string} bucketName The name of the bucket to list caches for.
- Example: 'your-gcp-bucket-name'
*/
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.
Thank you for your thoughtful suggestion and for providing such a well-crafted JSDoc comment. We aim to maintain a consistent documentation style across all our Cloud Storage samples to offer a unified experience for our users.
Adopting a new format, even if it is an improvement, could impact the overall cohesiveness and clarity of the sample set.
}; | ||
|
||
// Run request | ||
const [response] = await controlClient.listAnywhereCaches(request); |
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.
might wrap the response in a try.. catch
if there are no Anywhere Caches found for the bucket versus errors listing Anywhere Caches
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.
However, our code samples are designed to demonstrate the simplest, "happy path" usage of a feature. We intentionally omit error handling to keep the code concise and focused on the core API call. This approach ensures all Cloud Storage samples share a consistent, minimal style, making them easier to understand at a glance.
|
||
function main(bucketName, cacheName, admissionPolicy) { | ||
// [START storage_control_update_anywhere_cache] | ||
/** |
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.
nit:
/**
- Updates the Admission Policy of an Anywhere Cache instance.
- @param {string} bucketName The name of the bucket where the cache resides.
- Example: 'your-gcp-bucket-name'
- @param {string} cacheName The unique identifier of the cache instance to update.
- Example: 'my-anywhere-cache-id'
- @param {string} admissionPolicy Determines when data is ingested into the cache
*/
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.
Thank you for your thoughtful suggestion and for providing such a well-crafted JSDoc comment. We aim to maintain a consistent documentation style across all our Cloud Storage samples to offer a unified experience for our users.
Adopting a new format, even if it is an improvement, could impact the overall cohesiveness and clarity of the sample set.
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. |
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.