feat: component exclusion config #128
Conversation
| @@ -354,6 +355,16 @@ private final class Sdk( | |||
| } | |||
| } | |||
|
|
|||
| private def isNotExcluded(clz: Class[_]): Boolean = { | |||
There was a problem hiding this comment.
could we avoid double negation here?
isExcluded and then
componentClasses
.filter(hasComponentId)
.filterNot(isExcluded)
There was a problem hiding this comment.
please, filterNot(i => !i.isNotExcluded) 😄
| @@ -546,6 +558,7 @@ private final class Sdk( | |||
| private val viewDescriptors: Seq[SpiViewDescriptor] = | |||
There was a problem hiding this comment.
why isn't viewDescriptors covered by above foreach in the same way as other components?
as it is now, views will be logged as "Unknown component"?
| @@ -546,6 +558,7 @@ private final class Sdk( | |||
| private val viewDescriptors: Seq[SpiViewDescriptor] = | |||
| componentClasses | |||
| .filter(hasComponentId) | |||
| .filter(isNotExcluded) | |||
| .collect { | |||
| case clz if classOf[View].isAssignableFrom(clz) => ViewDescriptorFactory(clz, serializer, sdkExecutionContext) | |||
| } | |||
There was a problem hiding this comment.
what about HttpEndpoints? shouldn't those also be possible to exclude?
There was a problem hiding this comment.
good point, added filtering there
# Conflicts: # akka-javasdk/src/main/scala/akka/javasdk/impl/SdkRunner.scala
| @@ -1,2 +1,4 @@ | |||
| # Using a different port to not conflict with parallel tests | |||
| akka.javasdk.testkit.http-port = 39391 | |||
| akka.javasdk.profiles.active = test | |||
| @@ -36,4 +38,5 @@ private[impl] object Settings { | |||
| private[impl] final case class Settings( | |||
| cleanupDeletedEventSourcedEntityAfter: Duration, | |||
| cleanupDeletedKeyValueEntityAfter: Duration, | |||
| devModeSettings: Option[DevModeSettings]) | |||
| devModeSettings: Option[DevModeSettings], | |||
| excludedComponents: Seq[String]) | |||
| httpPort = sdkConfig.getInt("dev-mode.http-port")))) | ||
| httpPort = sdkConfig.getInt("dev-mode.http-port"))), | ||
| excludedComponents = | ||
| Option(sdkConfig.getString("components.exclude")).fold(Seq.empty[String])(_.split(",").toSeq)) |
There was a problem hiding this comment.
why is this Option fold needed? shouldn't be null as far as I can see
There was a problem hiding this comment.
could be good to .map(_.trim) the values in case they use space in the config
exclude = "akkajavasdk.components.keyvalueentities.user.ProdCounterEntity, akkajavasdk.components.keyvalueentities.user.StageCounterEntity"
There was a problem hiding this comment.
yeah, initially, I wanted to add protection against a null case, good hint with trim, added
| @@ -92,4 +92,6 @@ akka.javasdk { | |||
| collector-endpoint = ${?COLLECTOR_ENDPOINT} | |||
| } | |||
| } | |||
| # The comma separated list of FQCNs of components to exclude from running | |||
| components.exclude = "" | |||
There was a problem hiding this comment.
Should it be exclude or disable?
Both are good of course. I'm just trying to imagine if one is easier to understand then the other.
If we say exclude will people ask: exclude from what?
I don't know.
There was a problem hiding this comment.
disable or disabled ?
There was a problem hiding this comment.
disable as in imperative tense. Like in exclude.
| @@ -354,6 +355,16 @@ private final class Sdk( | |||
| } | |||
| } | |||
|
|
|||
| private def isExcluded(clz: Class[_]): Boolean = { | |||
| @@ -1,2 +1,3 @@ | |||
| # Using a different port to not conflict with parallel tests | |||
| akka.javasdk.testkit.http-port = 39391 | |||
| akka.javasdk.components.disabled = "akkajavasdk.components.keyvalueentities.user.ProdCounterEntity,akkajavasdk.components.keyvalueentities.user.StageCounterEntity" | |||
There was a problem hiding this comment.
is this used by any test? it is wrong.
disabled vs disable
There was a problem hiding this comment.
just noticed :) fixed
No description provided.