Uh oh!
There was an error while loading. Please reload this page.
feat: 30.2 — unified api surface - #4
Merged
Merged
Conversation
- Copy new service.proto: BatchEnqueue RPC removed, Enqueue now takes repeated EnqueueMessage, Ack/Nack take repeated messages with per-item results, ConsumeResponse uses only repeated messages field - Rename BatchEnqueueResult to EnqueueResult (no "batch" prefix) - Replace batchEnqueue() with enqueueMany() on FilaClient - Update enqueue() to wrap single message in repeated EnqueueMessage - Update ack()/nack() to wrap in repeated, parse first result with typed error handling (AckError/NackError) - Update Batcher to use unified Enqueue RPC for all batch sizes - Update consumeStream() to use only getMessagesList() (no singular message fallback) - Update all tests to use new API names and types
5 tasks
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/main/java/dev/faisca/fila/FilaClient.java">
<violation number="1" location="src/main/java/dev/faisca/fila/FilaClient.java:222">
P2: `ack()` treats missing/unset per-item results as success. Validate that exactly one result is returned and that it is an explicit success case.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Uh oh!
There was an error while loading. Please reload this page.
ack() and nack() silently treated empty results and RESULT_NOT_SET as success. Now both methods validate exactly one result is returned and that it is an explicit success case, matching the pattern in enqueueDirect().
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BatchEnqueueRPC removed;Enqueuenow acceptsrepeated EnqueueMessageAck/Nacknow acceptrepeated AckMessage/NackMessagewith per-item typed resultsConsumeResponseuses onlyrepeated Message messages(singularmessagefield removed)batchEnqueue()replaced byenqueueMany()(no "batch" prefix)BatchEnqueueResultrenamed toEnqueueResultEnqueueRPC for all batch sizesTest plan
Summary by cubic
Unifies the Java SDK and proto around a single enqueue/ack/nack API with per-item results and typed errors, removing the
BatchEnqueueRPC. AddsStreamEnqueue, simplifiesConsumeResponsetomessages[], renames client batch APIs toenqueueMany, and routes the batcher through the unifiedEnqueue.Bug Fixes
ack()/nack()now validate exactly one per-item result and require explicit success; empty or unset results throw.Migration
FilaClient.batchEnqueue(...)withFilaClient.enqueueMany(...); useEnqueueResult(wasBatchEnqueueResult).BatchEnqueue; useEnqueueRequest.messages[],AckRequest.messages[],NackRequest.messages[]; readConsumeResponse.messages[]only.EnqueueError,AckError,NackError.Written for commit 33358e2. Summary will update on new commits.