Skip to content
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

Add ConnectionChecker feature #190

Merged
merged 3 commits into from
Mar 16, 2021

Conversation

Arkatufus
Copy link
Contributor

  • Add ConnectionChecker
  • Add the ability to declare kafka settings directly inside of HOCON (declaring key and value serializer/deserializer is not implemented yet)

Copy link
Contributor

@IgorFedchenko IgorFedchenko left a comment

Choose a reason for hiding this comment

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

LGTM, but left one question about stopping actor instaed of letting parent stage to dicide what to do using supervision strategy.


var config = ConfigurationFactory.ParseString("akka.loglevel = DEBUG");
//var config = ConfigurationFactory.ParseString("akka.loglevel = DEBUG");
var config = ConfigurationFactory.ParseString("akka{}");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not to keep Debug level here? I guess this is helpful for debugging by default.

_log.Debug($"Received Stop from {Sender}, stopping");
Context.Stop(Self);
return true;

case KafkaConnectionFailed kcf:
ProcessError(kcf);
Self.Tell(KafkaConsumerActorMetadata.Internal.Stop.Instance);
Copy link
Contributor

Choose a reason for hiding this comment

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

So if there is a temporary disconnect from broker, there is no other way of handling that except stopping consuming? Right now this depends on supervision strategy, you can just try to resume. ProcessError method already sending failure notification to parent (source stage), and parent may stop this actor if Dicider is configured to do that.

@IgorFedchenko
Copy link
Contributor

Added an issue to update documentation #191

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.

2 participants