refactor: process ain't async #1180
Conversation
WalkthroughThis pull request simplifies error handling and processing by consolidating the asynchronous calls into a more concise, synchronous flow. The changes include merging two await statements into a single chained call in the Kafka processor, altering the Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as Ingest Handler
participant Event as Event Module
participant Kafka as Kafka Processor
Handler->>Event: process() (sync call)
Event-->>Handler: Return Result/Error
Kafka->>Event: build_event_from_chunk(&records)
Event-->>Kafka: Constructed Event
Kafka->>Event: process() (sync call)
Event-->>Kafka: Return Result/Error
Suggested reviewers
Poem
🪧 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/event/mod.rs (1)
49-85: LGTM! Consider adding documentation.The change from async to sync is appropriate as the method doesn't perform any asynchronous operations. All operations (schema key generation, partition handling, and stats updates) are synchronous.
Consider adding documentation to explain:
- The purpose of the method
- The significance of the schema key
- The impact of time and custom partitions
- The relationship with
process_unchecked
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/connectors/kafka/processor.rs(1 hunks)src/event/mod.rs(1 hunks)src/handlers/http/ingest.rs(1 hunks)src/handlers/http/modal/utils/ingest_utils.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- 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 aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
🔇 Additional comments (3)
src/connectors/kafka/processor.rs (1)
112-112: LGTM! Code is more concise while maintaining error handling.The change simplifies the code by combining the async
build_event_from_chunkwith the now-synchronousprocesscall, while preserving proper error propagation.src/handlers/http/modal/utils/ingest_utils.rs (1)
141-141: LGTM! Change aligns with the Event struct modifications.The synchronous
processcall is consistent with the Event struct changes and maintains proper error handling.src/handlers/http/ingest.rs (1)
107-107: LGTM! Change maintains consistency with Event struct modifications.The synchronous
processcall aligns with the Event struct changes while preserving proper error handling and the surrounding async context.
Fixes #XXXX.
Description
This PR has:
Summary by CodeRabbit
This update refines internal event handling, potentially contributing to better system stability and performance.