Skip to content

Conversation

@piccione99
Copy link
Contributor

@piccione99 piccione99 commented Nov 21, 2025

see KAFKA_IMPLEMENTION.md for changes. This file can be removed prior to merge.

Fixing AI generated garbage in the integration test.

@piccione99 piccione99 self-assigned this Nov 21, 2025
@piccione99 piccione99 marked this pull request as draft November 21, 2025 14:12
@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 85.65022% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.20%. Comparing base (fff641e) to head (d6ec2ab).

Files with missing lines Patch % Lines
publisher.go 89.40% 12 Missing and 4 partials ⚠️
main.go 0.00% 15 Missing ⚠️
eventDispatcher.go 97.36% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #562      +/-   ##
==========================================
+ Coverage   58.94%   62.20%   +3.26%     
==========================================
  Files          18       19       +1     
  Lines        1510     1717     +207     
==========================================
+ Hits          890     1068     +178     
- Misses        577      602      +25     
- Partials       43       47       +4     
Flag Coverage Δ
unittests 62.20% <85.65%> (+3.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@piccione99 piccione99 marked this pull request as ready for review December 3, 2025 16:21

// OnDeviceEvent is the device.Listener function that processes outbound events.
func (d *eventDispatcher) OnDeviceEvent(event *device.Event) {
d.logger.Debug("Received device event", zap.Any("event", event))
Copy link
Member

Choose a reason for hiding this comment

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

What is the performance/memory impact of this call? This is a very high throughput call & this may be totally fine or it may cause memory pressure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it will have an impact because of reflection so it's removed

publisher.go Outdated
// Convert wrp v3 message to v5 for wrpkafka
v5msg := convertV3ToV5(msg)

// *** find better way to do this - set QoS to Critical if in test mode so the message flushes immediately **
Copy link
Contributor Author

@piccione99 piccione99 Dec 5, 2025

Choose a reason for hiding this comment

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

Should never change production code for testing. Not sure how else to get the message to flush during an integration test. Setting a linger of 10ms still did not get the message to flush before the test timed out. @schmidtw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

goofy workaround is gone now. works ok now, unclear why it wasn't before. linger is also set back to 0.

@piccione99 piccione99 added the kafka add kafka streams to xmidt label Dec 9, 2025
@piccione99 piccione99 marked this pull request as draft January 8, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kafka add kafka streams to xmidt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants