Skip to content

Conversation

@Arkatufus
Copy link
Contributor

Fixes #7242

Changes

  • Make Akka.Discovery.CreateServiceDiscovery() also accept constructor with ExtendedActorSystem and Config parameters.
  • Remove Try<T> and use try..catch instead
  • Add unit tests

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

Comment on lines +115 to +121
ctor = type.GetConstructor(
bindingAttr: bindFlags,
binder: null,
types: new[] { typeof(ExtendedActorSystem) },
modifiers: null);
if (ctor is not null)
return (ServiceDiscovery) Activator.CreateInstance(type, _system);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still accepts constructor with a single ExtendedActorSystem parameter

Comment on lines +103 to +113
var ctor = type.GetConstructor(
bindingAttr: bindFlags,
binder: null,
types: new[]
{
typeof(ExtendedActorSystem),
typeof(Configuration.Config)
},
modifiers: null);
if (ctor is not null)
return (ServiceDiscovery) Activator.CreateInstance(type, _system, config);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New way to instantiate ServiceDiscovery that pushes the Config into the constructor

@Aaronontheweb Aaronontheweb added akka-delivery Akka.Delivery APIs akka-discovery Akka.Discovery-related issues configuration and removed akka-delivery Akka.Delivery APIs labels Jun 11, 2024
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb merged commit a14bb84 into akkadotnet:dev Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

akka-discovery Akka.Discovery-related issues configuration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Akka.Discovery less coupled with Akka.Management

2 participants