Skip to content
This repository has been archived by the owner on May 6, 2019. It is now read-only.

Prefer injected, typed config over Config #2

Open
ignasi35 opened this issue Oct 30, 2018 · 3 comments
Open

Prefer injected, typed config over Config #2

ignasi35 opened this issue Oct 30, 2018 · 3 comments

Comments

@ignasi35
Copy link
Contributor

AkkaDiscoveryServiceLocator uses the Config from the ActorSystem. Instead, it should receive a strongly typed custom configuration instance.

@dwijnand
Copy link
Contributor

dwijnand commented Nov 8, 2018

By "a strongly typed custom configuration instance" do you mean a custom case class, or one of those Scala wrappers of Config?

@ignasi35 ignasi35 changed the title Prefer injected, typed config over Prefer injected, typed config over Config Nov 8, 2018
@ignasi35
Copy link
Contributor Author

ignasi35 commented Nov 8, 2018

By "a strongly typed custom configuration instance" do you mean a custom case class, or one of those Scala wrappers of Config?

A custom case class.

@ignasi35
Copy link
Contributor Author

ignasi35 commented Feb 5, 2019

How about mapping the settings to something like:

case class LagomAkkaDiscoveryConfigDefaults(portName: Option[String],
                                            portProtocol: Option[String],
                                            scheme: Option[String])

case class ServiceNameMapping(lookup: Option[String],
                              scheme: Option[String])

case class LagomAkkaDiscoveryConfig(defaults: LagomAkkaDiscoveryConfigDefaults,
                                    serviceNameMappings: Map[String, ServiceNameMapping],
                                    lookupTimeout: Duration)

(just a proposal, we should not use case classes because of the bin compat issues)

Then:

- private[lagom] class ServiceNameMapper(config: Config) {
+ private[lagom] class ServiceNameMapper(config: LagomAkkaDiscoveryConfig) {

- private[lagom] class AkkaDiscoveryHelper(config: Config, serviceDiscovery: ServiceDiscovery)(
+ private[lagom] class AkkaDiscoveryHelper(config: LagomAkkaDiscoveryConfig, serviceDiscovery: ServiceDiscovery)(

etc...

Most of the complexity in ServiceMapper would move away from that class into a separate Config loading function.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants