raboof commented on code in PR #313:
URL: https://github.com/apache/pekko-management/pull/313#discussion_r1776874701
##########
discovery-kubernetes-api/src/main/scala/org/apache/pekko/discovery/kubernetes/KubernetesApiServiceDiscovery.scala:
##########
@@ -96,13 +96,14 @@ object KubernetesApiServiceDiscovery {
* An alternative implementation that uses the Kubernetes API. The main
advantage of this method is that it allows
* you to define readiness/health checks that don't affect the bootstrap
mechanism.
*/
-class KubernetesApiServiceDiscovery(implicit system: ActorSystem) extends
ServiceDiscovery {
+class KubernetesApiServiceDiscovery(settings: Settings)(
+ implicit system: ActorSystem) extends ServiceDiscovery {
import system.dispatcher
private val http = Http()
- private val settings = Settings(system)
+ def this()(implicit system: ActorSystem) = this(Settings(system))
Review Comment:
aha, I see `def this(implicit system: ActorSystem)` produces an "auxiliary
constructor needs non-implicit parameter list" error. Weird, I don't quite see
why.
Having those both isn't possible either, though, as they have the same
erasure (i.e. they lead to the same bytecode method signature).
I think having just `def this()(implicit system: ActorSystem) =
this(Settings(system))` is probably fine. While not entirely source compatible
(any `new KubernetesApiServiceDiscovery(system)` would need to be changed to
`new KubernetesApiServiceDiscovery()(system)`), we don't strictly promise
source compatibility and this class will typically be instantiated through
reflection or with an implicit `system` anyway.
It's binary compatible: comparing the `javap` signatures before and after
the only difference is the additional constructor:
```
22a23
> public
org.apache.pekko.discovery.kubernetes.KubernetesApiServiceDiscovery(org.apache.pekko.discovery.kubernetes.Settings,
org.apache.pekko.actor.ActorSystem);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For additional commands, e-mail: notifications-h...@pekko.apache.org
