shakeeb-upstart commented on code in PR #313:
URL: https://github.com/apache/pekko-management/pull/313#discussion_r1776671278
##########
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:
yes It will break binary compatibility, and the previous one broke source
compatibility, I suppose we would have to provide both constructors since Scala
doesn't support
`def this(implicit system: ActorSystem) = this(Settings(system))`
We will have to provide
`def this(system: ActorSystem) = this(Settings(system))` for binary
compatibility and
``def this()(implicit system: ActorSystem) = this(Settings(system))`` for
source compatibility
--
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
