Hi Colin,
Thanks for the quick response. Sure, the problem is that the internal classes (DescribeLogDirsResponse.[LogDirInfo, ReplicaInfo], or 'the old ones') are exposed to the public API, not the class itself. You are right. However, deprecating DescribeLogDirsResult#[all, values] and defining the new methods causes another problem: consistency. As of 2.5, most of the admin result classes (listed below) have 'all' and 'values' methods. Since these methods are public APIs, I think keeping consistency would be better. - https://kafka.apache.org/25/javadoc/org/apache/kafka/clients/admin/CreateTopicsResult.html - https://kafka.apache.org/25/javadoc/org/apache/kafka/clients/admin/AlterPartitionReassignmentsResult.html - https://kafka.apache.org/25/javadoc/org/apache/kafka/clients/admin/CreateAclsResult.html - https://kafka.apache.org/25/javadoc/org/apache/kafka/clients/admin/AlterConfigsResult.html In contrast, my approach - deprecating the old ones and defining the new ones (DescribeLogDirsResult.[LogDirInfo, ReplicaInfo]) that extends the old counterpart - not only not breaking the consistency but also gives the users the message that they have to use the new ones. So, the overall process would be: 1. Deprecate the old ones and make DescribeLogDirsResult#[all, values] to return the new ones, without changing the return type. Since the new ones extend the old ones, it does not cause any problems. 2. Since deprecation message guides to move to the new ones, the uses will migrate to use the new classes. 3. After a time, do the following: 1. Change the return type of DescribeLogDirsResult#[all, values] from old ones to the new ones, with a notice. Since we already deprecated the old ones, most users would already be moved into the new ones. So it does not occur any problems. 2. *Hide the old ones from the public, with removing the deprecated annotation.* Since it is not exposed to the public anymore, it also does not cause any problems - now we can use it freely in the internals. Is my intention clear now? If not, please have a brief look at my draft implementation here <https://github.com/apache/kafka/pull/7204/files>. Thanks, Dongjin On Thu, Jul 9, 2020 at 4:34 AM Colin McCabe <cmcc...@apache.org> wrote: > Hi Dongjin, > > Hmm. I'm not sure I follow. How does deprecating > DescribeLogDirsResponse.LogDirInfo help here? The issue is not so much the > class, but the fact that it's exposed as a public API. So it seems > appropriate to deprecate the methods that return it, but not the class > itself, since we'll continue to use it internally. > > best, > Colin > > > On Wed, Jul 8, 2020, at 07:41, Dongjin Lee wrote: > > Hi Tom, > > > > Thanks for taking this issue. I opened a PR for this issue earlier, but > > your KIP was submitted first. So I closed my one > > < > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158866169 > > > > . > > > > I have a question: for consistency with other methods, how about > > maintaining the signature of DescribeLogDirsResult#[all, values]? There > is > > an alternative approach to deprecate DescribeLogDirsResponse.[LogDirInfo, > > ReplicaInfo]. (Please have a look at my proposal.) > > > > Best, > > Dongjin > > > > +1. I updated the link to the discussion thread on your KIP document. > > > > On 2020/06/29 09:31:53, Tom Bentley <t...@redhat.com> wrote: > > > Hi,> > > > > > > Does anyone have any comments about this? If not I'll likely start a > > vote> > > > in a couple of days.> > > > > > > Kind regards,> > > > > > > Tom> > > > > > > On Mon, Jun 8, 2020 at 4:56 PM Tom Bentley <tb...@redhat.com> wrote:> > > > > > > > Hi all,> > > > >> > > > > I've opened a small KIP seeking to deprecate and replace a couple of> > > > > methods of DescribeLogDirsResult which refer to internal classes in > > their> > > > > return type.> > > > >> > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158862109 > > > > > >> > > > > Please take a look if you have the time.> > > > >> > > > > Kind regards,> > > > >> > > > > Tom> > > > >> > > > > > > -- *Dongjin Lee* *A hitchhiker in the mathematical world.* *github: <http://goog_969573159/>github.com/dongjinleekr <https://github.com/dongjinleekr>keybase: https://keybase.io/dongjinleekr <https://keybase.io/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: speakerdeck.com/dongjin <https://speakerdeck.com/dongjin>*
