Have you created the ticket or KIP for deprecating time-accepting constructors?
thanks On 2026/04/23 20:57:19 Siddhartha Devineni wrote: > Hi Chia-Ping, Kirk and Matthias, > > @Chia-Ping: You were right - after investigating, Time doesn't need to be > public. > > @Kirk: You are right - the OAuth examples are instantiated via reflection, > not direct user code. > > @Matthias: Good points. I investigated whether Metrics can be changed to > not expose Time. > > Findings: > > - new Metrics() - users call this (internally uses Time.SYSTEM) > - new Metrics(Time time) and other variants - only called by internal Kafka > code (KafkaProducer, KafkaAdminClient, etc) and tests > > Proposed approach: > > - Withdraw KIP-1311 (Make Time public) > - Create JIRA: "Deprecate Time-accepting constructors" > - Deprecate Time constructors in both KafkaStreams and Metrics: > - KafkaStreams(Topology, Properties, Time) > - KafkaStreams(Topology, StreamsConfig, Time) > - KafkaStreams(Topology, Properties, KafkaClientSupplier, Time) > - Metrics(Time) > - Metrics(MetricConfig, Time) > - Metrics(MetricConfig, List<MetricsReporter>, Time) > - (and other Metrics variants accepting Time) > - In version 5.0, make these constructors package-private > - Internal Kafka code continues using them > > Result: Time remains internal. > > Does this approach work? If so, I'll withdraw KIP-1311 and create the JIRA. > > Thank you, > Siddhartha > > On Tue, Apr 21, 2026 at 5:22 AM Matthias J. Sax <[email protected]> wrote: > > > Thanks for the KIP. I am not sure if I understand why `KafkaStreamsMock` > > would be anything public? > > > > Also, why would we put it into some new `...test...` package? If we > > change the package, we need to have `protected` access, which is already > > "semi-public"... > > > > If we want to keep `Time` internal, we would eventually make the > > constructors that are marked deprecated, package-private, what allows us > > to add `org.apache.kafka.streams.KafkaStreamsMock` (same package name, > > but int `test/` module) to still use these constructors, and the > > corresponding unit test would use the new mock-factory instead of > > calling `new`? > > > > For this case, the KIP does not need to mention anything about > > `KafkaStreamsMock` as it's an helper in our `test/` module only, but not > > public API. -- If we want, we can still mention this plan on the KIP, > > but atm the KIP is written in a way as if `KafkaStreamsMock` would > > become public API, but to my understanding it should be an impl/testing > > details only? > > > > Or did I misunderstand something? > > > > > > Also wondering, if we could also change `Metrics` in a way, that we > > would not need to make `Time` public to begin with? > > > > > > > > -Matthias > > > > On 4/20/26 4:53 PM, Kirk True wrote: > > > Hi Siddhartha, > > > > > > The OAuth examples use Time in their constructors for unit tests. > > They're not intended to be instantiated by any user code since they're in > > an internals package. > > > > > > Thanks, > > > Kirk > > > > > > On Wed, Apr 15, 2026, at 8:52 AM, Siddhartha Devineni wrote: > > >> Hi Chia-Ping, > > >> > > >> Sorry that i didn't mention the following examples in the KIP earlier. > > >> Now, I have updated the KIP with the following public packages examples, > > >> where "Time" is exposed in the public constructors: > > >> > > >> // couple of examples from multiple > > >> "org.apache.kafka.common.metrics.Metrics.java" constructors > > >> public Metrics(Time time) {} > > >> public Metrics(MetricConfig defaultConfig, Time time) {} > > >> > > >> // in the public package "org.apache.kafka.common.security.oauthbearer" > > >> public JwtBearerJwtRetriever(Time time) {} > > >> public ClientCredentialsJwtRetriever(Time time) {} > > >> > > >> Now, it should be clear. > > >> Thanks for your time. > > >> > > >> Best regards, > > >> Siddhartha > > >> > > >> On Tue, Apr 14, 2026 at 11:14 AM Chia-Ping Tsai <[email protected]> > > wrote: > > >> > > >>> hi Siddhartha > > >>> > > >>> Thanks for this KIP. > > >>> > > >>> What is the exact benefit of exposing Time as a public API? Since this > > KIP > > >>> proposes deprecating KafkaStreams(Topology, Properties, Time), it seems > > >>> there are no public interfaces relying on it anymore. > > >>> > > >>> Thus, it should be fine to just keep Time as an internal API, right? > > >>> > > >>> Best, > > >>> Chia-Ping > > >>> > > >>> > > >>> Siddhartha Devineni <[email protected]> 於 2026年4月7日週二 > > >>> 下午2:19寫道: > > >>> > > >>>> Apologies, as I forgot to add the link to the KIP: > > >>>> > > >>>> > > >>> > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=406623925 > > >>>> > > >>>> > > >>>> On Tue, Apr 7, 2026 at 9:13 AM Siddhartha Devineni < > > >>>> [email protected]> wrote: > > >>>> > > >>>>> Hello everyone, > > >>>>> > > >>>>> I would like to start a discussion on [DISCUSS] KIP-1311: Make > > >>> Time/Timer > > >>>>> public API. > > >>>>> > > >>>>> Following KIP-1247 (Make Bytes part of public API), the Time > > interface > > >>>> and > > >>>>> Timer class are the next candidates from > > >>> "org.apache.kafka.common.utils" > > >>>> to > > >>>>> be made officially public. Time is currently exposed through public > > >>> APIs > > >>>>> (e.g., in clients, KafkaStreams constructors, etc) but not officially > > >>>>> designated as a public API. > > >>>>> > > >>>>> An earlier version of this KIP explored splitting Time into focused > > >>>>> interfaces (Clock, MonotonicClock, etc.), but this would require > > >>>> rewriting > > >>>>> thousands of method signatures across the Kafka codebase. The simpler > > >>>>> approach of making Time public as-is seems more appropriate to avoid > > >>>>> breaking changes. > > >>>>> > > >>>>> Looking forward to your feedback. > > >>>>> > > >>>>> Thank you, > > >>>>> Siddhartha > > >>>>> > > >>>> > > >>> > > >> > > > > > > > >
