Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-06-13 Thread via GitHub
phooq closed pull request #15750: KAFKA-16557: Fix toString of OffsetFetchRequestState URL: https://github.com/apache/kafka/pull/15750 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific co

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-25 Thread via GitHub
kirktrue commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2078159039 I think I would limit it to the request state classes, just to keep the changes to a minimum -- This is an automated message from the Apache Git Service. To respond to the message, ple

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-25 Thread via GitHub
phooq commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2078128947 Thanks for the suggestion @kirktrue . I will use `toStringDetails()` then. Would you suggest I change the methods in the `***Event` classes as well, or just focus on `RequestState` for this

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-25 Thread via GitHub
kirktrue commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077701719 What about `toStringDetails()`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specif

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-25 Thread via GitHub
kirktrue commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077634416 Thanks for the work on this @phooq! > I plan to, on top the current changes I have, rename the `toStringBase` method as `getDetails` Renaming `toStringBase()` to `getDetails

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-25 Thread via GitHub
phooq commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077592945 Thanks so @lianetm ! Hey @kirktrue , I plan to, on top the current changes I have, rename the `toStringBase` method as `getDetails` for `RequestState`, does this look okay to you? T

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-25 Thread via GitHub
lianetm commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2077166262 Hey @phooq, having a `getDetails` that the inheritors override does achieve the clarity I was looking for, sounds good. -- This is an automated message from the Apache Git Service. To

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-19 Thread via GitHub
phooq commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2066909600 Thanks for the feedback @lianetm ! I agree with the point about the back-and-forth jumping between the child and the parent coming with this implementation, however asking each child

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-19 Thread via GitHub
lianetm commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2066727111 Thanks for the info @kirktrue , I see you mentioned it was a "suggested pattern", and I won't hold a strong position here but still sharing how I see it (struggling to find a value in it

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-18 Thread via GitHub
phooq commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2065444919 Hey @lianetm , @kirktrue Thanks for the feedback and suggestions! I like @kirktrue 's advice on having a `toString()` override at the base class while each subclass customized the `to

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-18 Thread via GitHub
kirktrue commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2065387420 We've been encouraged to follow the pattern of having the super class implement `toString()` and add a `toStringBase()`, like this: ```java public class Parent { priv

Re: [PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-18 Thread via GitHub
lianetm commented on PR #15750: URL: https://github.com/apache/kafka/pull/15750#issuecomment-2064223926 Hey @phooq, thanks for taking on this one. High level question about the motivation. The `toStringBase` defined in the base class `RequestState` includes vars that all states have, so tha

[PR] KAFKA-16557: Fix toString of OffsetFetchRequestState [kafka]

2024-04-17 Thread via GitHub
phooq opened a new pull request, #15750: URL: https://github.com/apache/kafka/pull/15750 ### Description This PR is for [KAFKA-16557](https://issues.apache.org/jira/browse/KAFKA-16557). This is to enhance the debugging and troubleshooting for consumer related issues. ### Test