fix: datasets API edgecase where stream is not hottiered #1252
Conversation
WalkthroughThe changes update the error handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant D as PrismDatasetRequest
participant H as HotTierManager
C->>D: Call get_datasets()
D->>H: Call get_hot_tier()
alt Success
H-->>D: Ok(stats)
D-->>C: Some(stats)
else NotFound Error
H-->>D: Err(NotFound)
D-->>C: None
else Other Error
H-->>D: Err(error)
D-->>C: Error propagated
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/prism/logstream/mod.rs (1)
277-283: Improved error handling for hottier edge caseThe implementation now properly handles the edge case where the hottier is not configured at the stream level but is enabled at the instance level. By explicitly handling the
NotFounderror, the code now returnsNonefor streams without hottier configuration instead of propagating the error.Consider adding a comment explaining this specific edge case for future maintainers:
let hottier = match HotTierManager::global() { + // Handle case where hottier is enabled at instance level but not configured for this stream Some(manager) => match manager.get_hot_tier(stream).await { Ok(stats) => Some(stats), Err(HotTierError::HotTierValidationError( HotTierValidationError::NotFound(_), )) => None, Err(err) => return Err(err.into()), }, _ => None, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/prism/logstream/mod.rs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (1)
src/prism/logstream/mod.rs (1)
48-48: LGTM: Import added for new error handlingThis import is correctly added to support the more granular error handling in the match statement below.
Fixes #XXXX.
Description
Handles the circumstance where hottier is not configured at the stream level, but hottier is enabled at instance level
This PR has:
Summary by CodeRabbit