feat(go): add delete segments support#3191
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3191 +/- ##
=========================================
Coverage 74.10% 74.10%
Complexity 943 943
=========================================
Files 1159 1161 +2
Lines 102033 102048 +15
Branches 79083 79083
=========================================
+ Hits 75607 75622 +15
Misses 23765 23765
Partials 2661 2661
🚀 New features to boost your workflow:
|
| func TestIggyTcpClient_DeleteSegments(t *testing.T) { | ||
| clientConn, serverConn := net.Pipe() | ||
| defer func() { | ||
| _ = clientConn.Close() | ||
| }() |
There was a problem hiding this comment.
My concern is that this specific test doesn't quite align with our current patterns, as most tests requiring a server live in the /bdd directory.
Long-term, I think we should move this type of testing to /bdd and provide a common scenario for every SDK to implement.
I'm also curious to hear what others think about moving this to BDD. Maybe we can add extra steps in basic_messaging.feature?
There was a problem hiding this comment.
I removed the unit test and moved the behavioral coverage into /bdd, matching how the rest of the Go TCP suite exercises the SDK against a live server.
| } | ||
|
|
||
| func (d *DeleteSegments) MarshalBinary() ([]byte, error) { | ||
| bytes, _ := iggcon.MarshalIdentifiers(d.StreamId, d.TopicId) |
There was a problem hiding this comment.
This error should not be ignored.
There was a problem hiding this comment.
I did it since it's unreachable in tests, I'll return it
chengxilo
left a comment
chengxilo
left a comment
There was a problem hiding this comment.
Overall, a solid PR! I left some comment, regarding the test, I'm curious about you and others' opinion.
Expose the delete segments command in the Go SDK so clients can manage partition segment cleanup over TCP.
Add focused coverage for the TCP delete segments wrapper and command code while simplifying payload serialization through the shared identifier helper.
Explicitly ignore pipe close errors in the delete segments TCP test so Go lint passes under CI.
Avoid an impossible identifier serialization branch in delete segments payload encoding so patch coverage reflects the exercised command behavior.
Cover delete segments through the Go TCP BDD suite so server-backed SDK behavior follows the existing test layout.
matanper
force-pushed
the
go-delete-segments-sdk
branch
from
f4169c7 to
301398c
Compare
|
lgtm |
2 participants
Expose the delete segments command in the Go SDK so clients can manage partition segment cleanup over TCP.
Which issue does this PR close?
Closes #3190
Rationale
The Go SDK was missing support for the existing
DeleteSegmentsTCP command, so Go clients could not trigger partition segment cleanup through the SDK.What changed?
The Go SDK now exposes
DeleteSegments(streamId, topicId, partitionId, segmentsCount), adds the TCP command code and binary payload serialization, and includes a serialization test for the command wire format.Local Execution
go test ./...fromforeign/goapache/iggy:0.8.0on TCP, including login, stream/topic creation, message send,DeleteSegments, topic fetch, and cleanupAI Usage