Make admin-client java converters internal #1567
Conversation
The admin client re-defines java classes in their Scala equivalent. The scala ↔ java converters are part of these classes even though they should not be used, or even not be needed, by users of the admin client. In this change the converter code is removed from the public API and moved to an internal package. The converter code is still available by importing the internal package. However, please note that code in internal packages may change in any way in any zio-kafka release. ``` import zio.kafka.admin.internal.JavaConverters._ ```
| ReplicaInfo(size = jri.size(), offsetLag = jri.offsetLag(), isFuture = jri.isFuture) | ||
| } | ||
|
|
||
| implicit class AclOperationAsScala(private val ao: AclOperation) extends AnyVal { |
There was a problem hiding this comment.
Shouldn't the AclOperationAsScala and JAclOperationAsJava names be switched here?
| } | ||
|
|
||
| implicit class ListConsumerGroupsOptionsAsJava(private val lcgo: ListConsumerGroupsOptions) extends AnyVal { | ||
| def asJListGroupsOptions: JListGroupsOptions = |
| } | ||
|
|
||
| implicit class JTopicDescriptionAsScala(private val td: JTopicDescription) extends AnyVal { | ||
| def asScala: Try[TopicDescription] = { |
There was a problem hiding this comment.
This was present in the refactored code.
| * may be null | ||
| */ | ||
| implicit class JNodeAsScala(private val jn: JNode) extends AnyVal { | ||
| def asScala: Option[Node] = |
There was a problem hiding this comment.
Since every java object (or scala object in fact) can be null, it would be consistent to make all asScala operations return an Option. Or not and do an Option(..).map(_.asScala) at the calling location. The second would be my preference.
| } | ||
| final case class ConfigResource(`type`: ConfigResourceType, name: String) | ||
|
|
||
| sealed trait ConfigResourceType |
There was a problem hiding this comment.
This is a lot cleaner now.
| case object Subtract extends AlterConfigOpType | ||
| // noinspection SpellCheckingInspection | ||
| @deprecated(since = "3.1.0", message = "Use Subtract") | ||
| val Substract: Subtract.type = Subtract |
There was a problem hiding this comment.
I often make this typo too.
|
@svroonland I propose we close this issue without merging. I like it that the code moves out of the public API, but I do not like it that this is a large refactoring without much tests, nor do I like how this adds method |
|
Sure, I can go along with that. |
2 participants
The admin client re-defines java classes in their Scala equivalent. The scala ↔ java converters are part of these classes even though they should not be used, or even not be needed, by users of the admin client.
In this change the converter code is removed from the public API and moved to an internal package.
The converter code is still available by importing the internal package. However, please note that code in internal packages may change in any way in any zio-kafka release.