Skip to content

Conversation

@piccione99
Copy link
Contributor

@piccione99 piccione99 commented Sep 4, 2025

also added kinesis and batch packages, but they aren't used yet, please ignore them for reviewing.

OutboundSender contains the common functionality for WebhookOutboundSender and eventually StreamOutboundSender. t

However, OutboundSender has the Update function that takes ancla.Webhook so it doesn't really fit that well with StreamOutboundSender. So, that is a code smell that probably needs to be addressed. That said, we might want to actually update stream senders from Argus so we don't have to reboot caduceus. Initially this will be a predefined set of streams and we need to manually add them to headwaters, so it can't be entirely automated. But would be nice to not have to redeploy caduceus.

@codecov
Copy link

codecov bot commented Sep 5, 2025

Codecov Report

❌ Patch coverage is 64.01180% with 244 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.73%. Comparing base (d496c94) to head (47cbc87).

Files with missing lines Patch % Lines
internal/kinesis/kinesis.go 41.74% 114 Missing and 6 partials ⚠️
webhook_dispatcher.go 72.62% 40 Missing and 9 partials ⚠️
webhook_outbound_sender.go 72.27% 22 Missing and 6 partials ⚠️
main.go 0.00% 21 Missing ⚠️
stream_dispatcher.go 80.30% 10 Missing and 3 partials ⚠️
internal/stream/sender.go 67.85% 6 Missing and 3 partials ⚠️
stream_outbound_sender.go 89.28% 2 Missing and 1 partial ⚠️
outboundSender.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #614      +/-   ##
==========================================
+ Coverage   53.89%   55.73%   +1.83%     
==========================================
  Files           9       16       +7     
  Lines        1334     1830     +496     
==========================================
+ Hits          719     1020     +301     
- Misses        574      749     +175     
- Partials       41       61      +20     
Flag Coverage Δ
unittests 55.73% <64.01%> (+1.83%) ⬆️

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 requested a review from schmidtw September 5, 2025 13:42
@piccione99 piccione99 marked this pull request as draft September 5, 2025 13:43
dispatcher.go Outdated
Send(urls *ring.Ring, secret, acceptType string, msg *wrp.Message)
}

func DispatcherFactory(webhook bool, obs *CaduceusOutboundSender) Dispatcher {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is removed

}

obs.(*CaduceusOutboundSender).queueOverflow()
obs.(*WebhookOutboundSender).obs.dispatcher.QueueOverflow()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will eventually also return StreamOutboundSender

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.

1 participant