Re: [PR] MINOR: fully encapsulate user restore listener in the DelegatingRestoreListener [kafka]

2023-12-06 Thread via GitHub


ableegoldman merged PR #14886:
URL: https://github.com/apache/kafka/pull/14886


-- 
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 comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINOR: fully encapsulate user restore listener in the DelegatingRestoreListener [kafka]

2023-12-05 Thread via GitHub


ableegoldman commented on PR #14886:
URL: https://github.com/apache/kafka/pull/14886#issuecomment-1841644618

   Lots of test failures, but all are unrelated


-- 
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 comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[PR] MINOR: fully encapsulate user restore listener in the DelegatingRestoreListener [kafka]

2023-11-30 Thread via GitHub


ableegoldman opened a new pull request, #14886:
URL: https://github.com/apache/kafka/pull/14886

   Minor cleanup to make it easier to follow the restore listener logic. 
Currently, the KafkaStreams class tracks two restore listener  fields: there is 
a non-final, nullable "globalRestoreListener" that holds the restore listener 
specified by the user (if any), and then there is a final 
"delegatingRestoreListener" that's used to encapsulate the null checks for the 
user-specified restore listener. It's a bit confusing to follow along with what 
each of these restore listener fields is doing and the relationship between 
them when they're on equal footing like this, when in reality they're more 
hierarchical and the DelegatingRestoreListener is actually a wrapper over the 
user-specified globalRestoreListener. The term "global" is also a bit 
misleading as it can get mixed up with global state stores, when it's really 
meant to be "global" in the sense that it applies to all state stores in the 
application. 
   
   It would be nice to just move the user listener completely inside the 
DelegatingRestoreListener class and then make that class static, as well as 
renaming the field to "userRestoreListener"


-- 
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 comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org