-
Notifications
You must be signed in to change notification settings - Fork 874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow multiple calls to SetStatisticsHandler / SetLogHandler / SetErrorHandler #2155
base: master
Are you sure you want to change the base?
Allow multiple calls to SetStatisticsHandler / SetLogHandler / SetErrorHandler #2155
Conversation
cf970f9
to
254ccfc
Compare
@forlack: This PR would be useful for dotnet/aspire#951 and open-telemetry/opentelemetry-dotnet-contrib#1493. Please see details in related issue #2154 |
2694696
to
00a8164
Compare
LogToFile("end ConsumerBuilder_SetStatisticsHandler"); | ||
} | ||
|
||
[SkipWhenCITheory("Requires to stop the broker in the while loop to simulate broker is down."), MemberData(nameof(KafkaParameters))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand. This requires someone to manually stop the broker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. this is required during the unit test to get the error handler called.
var record = consumer.Consume(TimeSpan.FromMilliseconds(100)); | ||
if (record == null) { continue; } | ||
if (record.IsPartitionEOF) { break; } | ||
msgCnt += 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to assert how many messages were counted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No that's not required here. good catch, I dropped the useless msgCnt
variables.
6e1e71e
to
e5552df
Compare
Resolves #2154