update: remove time partition stream creation #1227
Conversation
gives error if user tries to create a stream with a time partition updating time partition limit is still allowed for older streams
WalkthroughThe pull request modifies error handling and control flow in the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant P as Parseable
participant S as Stream Creator
C->>P: create_update_stream(headers, body, stream_name)
alt Time partition provided<br>(time_partition or limit non-empty)
P-->>C: Return Error (BAD_REQUEST)
else No time partition provided
P->>S: create_stream(None, ...)
S-->>P: Stream created
P-->>C: Return created stream
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
🪧 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/parseable/mod.rs (1)
465-470: Consider removing redundant checks for time partitioningSince creating streams with time partitioning is now completely disallowed (as enforced by lines 454-459), this check for both time partition and custom partition being set seems redundant.
-if !time_partition.is_empty() && custom_partition.is_some() { - return Err(StreamError::Custom { - msg: "Cannot set both time partition and custom partition".to_string(), - status: StatusCode::BAD_REQUEST, - }); -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/parseable/mod.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
🔇 Additional comments (3)
src/parseable/mod.rs (3)
454-459: Implementation correctly blocks time partition creationThe added code block effectively implements the PR objective to remove the functionality for creating new streams with time partitions, while providing a clear error message to users.
483-483: Consistent change for time partition removalReplacing the previous time partition value with
Noneis consistent with the change to remove time partition functionality for new stream creation.
472-479:❓ Verification inconclusive
Verify schema validation with time partition changes
The
validate_static_schemafunction still accepts atime_partitionparameter. Since time partition is no longer supported for new stream creation, make sure this doesn't cause issues.
🏁 Script executed:
#!/bin/bash # Examine how validate_static_schema uses the time_partition parameter ast-grep --pattern 'fn validate_static_schema($$$)' | grep -A 30 "validate_static_schema" # Check if time_partition is used in convert_static_schema_to_arrow_schema rg "convert_static_schema_to_arrow_schema" -A 2 -B 2Length of output: 1246
Action Required: Revisit Time Partition Handling in Schema Validation
- The function
validate_static_schemain src/parseable/mod.rs still passes thetime_partitionparameter toconvert_static_schema_to_arrow_schema(defined in src/static_schema.rs).- Since time partition is no longer supported for new stream creation, please verify that its continued use does not introduce unintended side effects. If the parameter is only relevant for legacy streams, consider conditionally bypassing or documenting its behavior for new stream scenarios.
…)" This reverts commit 10aef7b.
gives error if user tries to create a stream with a time partition
updating time partition limit is still allowed for older streams
Summary by CodeRabbit
Bug Fixes
Refactor