-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore: add snippets for dev docs, corrected some docstring examples in cache client #253
Conversation
#momento = "0.15.0" | ||
momento = { path = "../" } |
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.
examples are currently using the local SDK, but once we cut a release we can update this here and probably find a way to verify examples in CI
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.
Thanks for doing this. As a discussion point, I see two things:
- In the examples should we use the ?-operator to propagate errors up from the client calls?
- Should we test for particular error cases in some examples?
To illustrate, in some snippets we use the ?-operator on the cache calls, then display the answer in the happy case.
On others we match on the result and display an error.
On others we test for particular error codes, eg:
match cache_client.set(cache_name, "key", "value").await {
Ok(_) => println!("Set successful"),
Err(e) => {
if let MomentoErrorCode::NotFoundError = e.error_code {
println!("Cache not found: {}", cache_name);
} else {
eprintln!("Error setting value in cache {}: {}", cache_name, e);
}
}
}
My inclination given Rust's features is to:
- Bubble up the errors using
?
- Match on all the other cases (hit vs miss, stored vs not stored, cache created vs already exists)
We can socialize the options with the team during the bug bash.
cd75851
to
fe0b18e
Compare
…ngs like CacheClient methods
fe0b18e
to
2a430d7
Compare
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.
Couple small issues. Looks great otherwise and much improved and cleaner.
Addresses #210