Status

Current state: Under Discussion

Discussion thread: here

JIRA: KAFKA-13285

Please keep the discussion on the mailing list rather than commenting on the wiki (wiki discussions get unwieldy fast).

Motivation

This KIP is a 3rd attempt to bring consistency and testability to the Admin client's *Result classes. It follows two KIPs which were not accepted:

  • KIP-692 which proposed to make all the constructors public on the grounds of making it easier to users to write tests with mock Admin instances.
  • KIP-774 which proposed to deprecate the existing public constructors (most are currently package-private) and eventually revert them all to package-private, on the grounds of making it easier to evolve the overall Admin API in a compatible way.

The proposal this time is to establish the following convention/pattern for the instantiation of Result classes:

  • Use public access initially.
  • To evolve using constructor overloading and deprecation where possible (for example, when an API change requires adding a new constructor parameter in addition to an existing one we could deprecate the old constructor delegating to a new constructor passing null for the argument to the added parameter).
  • Use public static factory methods for API evolution that cannot use overloading (for example if an API change requires changing the type arguments of an existing constructor parameter, which would be impossible to do via constructor overloading while retaining compatibility because the erased signatures of the old and new constructors would be the same).

It is also proposed to:

  • Remove the final modifier from ElectLeadersResult to improve consistency.
  • Add a static KafkaFuture.failedFuture(Throwable) method for creating failed futures without needing to use the private KafkaFutureImpl class (KafkaFuture already has a static succeededFuture(T) method).

Public Interfaces

  • Change the access modifier of all existing non-deprecated package-private and/or protected constructors of *Result  classes in the Admin client to public.
  • Remove any final modifiers on those classes
  • Add KafkaFuture.failedFuture(Throwable)

/**
* Returns a new KafkaFuture that is already failed with the given exception.
*/
public static <U> KafkaFuture<U> failedFuture(Throwable cause)

Proposed Changes

Per public interfaces section.

Compatibility, Deprecation, and Migration Plan

This change is source and binary compatible with previous releases.

Rejected Alternatives

  • Do nothing.
  • Follow proposal of KIP-692.
  • Follow proposal of KIP-774.
  • Change the Result classes to interfaces. This would be quite a bit more code, for similar protection as a package-private constructor and not improve the situation for people mocking the Admin client.
  • No labels