Skip to content

fix(go,csharp): remove duplicate consumer groups in DeserializeClient/MapClient#3164

Merged
hubcio merged 3 commits into
apache:masterfrom
atharvalade:fix/go-sdk-deserialize-client-duplicate-consumer-groups
Apr 27, 2026
Merged

fix(go,csharp): remove duplicate consumer groups in DeserializeClient/MapClient#3164
hubcio merged 3 commits into
apache:masterfrom
atharvalade:fix/go-sdk-deserialize-client-duplicate-consumer-groups

Conversation

@atharvalade
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #3131

Rationale

DeserializeClient (Go) and MapClient (C#) return duplicate/zero-valued consumer groups due to incorrect slice pre-allocation and a redundant outer loop.

What changed?

In Go, make([]ConsumerGroupInfo, Count) pre-allocated Count zero-valued entries, then append added real data after them—doubling the slice. The outer for position < length loop could also re-read groups if trailing bytes existed.

Fixed both SDKs by using make([], 0, Count) (Go) / new List<>(Count) (C#) for correct capacity-only allocation, and replaced the outer while/for loop with a single bounded for i < Count pass.

Local Execution

  • Passed
  • Pre-commit hooks ran

AI Usage

  1. Opus 4.6
  2. Minimal AI used
  3. Verified via go build, go vet, go test, dotnet build, dotnet test (100/100 passed), and CI lint scripts
  4. Yes, all code can be explained

@atharvalade atharvalade changed the title fix(go,csharp): remove duplicate consumer groups in DeserializeClient… fix(go,csharp): remove duplicate consumer groups in DeserializeClient/MapClient Apr 24, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 56.00000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.77%. Comparing base (7de9d09) to head (149c31a).

Files with missing lines Patch % Lines
...nary_serialization/binary_response_deserializer.go 0.00% 11 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3164      +/-   ##
============================================
- Coverage     73.61%   72.77%   -0.85%     
  Complexity      943      943              
============================================
  Files          1141     1117      -24     
  Lines        100869    96105    -4764     
  Branches      78043    73303    -4740     
============================================
- Hits          74254    69939    -4315     
+ Misses        23958    23605     -353     
+ Partials       2657     2561      -96     
Components Coverage Δ
Rust Core 73.55% <ø> (-1.03%) ⬇️
Java SDK 62.30% <ø> (ø)
C# SDK 69.11% <100.00%> (-0.30%) ⬇️
Python SDK 81.43% <ø> (ø)
Node SDK 91.40% <ø> (-0.04%) ⬇️
Go SDK 39.43% <0.00%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
foreign/csharp/Iggy_SDK/Mappers/BinaryMapper.cs 91.69% <100.00%> (-1.10%) ⬇️
...nary_serialization/binary_response_deserializer.go 83.57% <0.00%> (+0.40%) ⬆️

... and 86 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@lukaszzborek lukaszzborek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve for csharp
for go looks the same 😅, but not sure about how tests looks like. Now there is no tests for that part

@hubcio hubcio merged commit e1b1b22 into apache:master Apr 27, 2026
61 of 77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

go-sdk: DeserializeClient returns duplicate consumer groups

5 participants