apoorvmittal10 commented on PR #22601: URL: https://github.com/apache/kafka/pull/22601#issuecomment-4798696026
> > @smjn I was expecting a single readLog method in LogReader which abstracts whether to fetch from local log or remote, it just provides a CompletableFuture. Do you think that can be achieved instead of two methods in LogReader? > > @apoorvmittal10 The current `read` method needs to stay until `DelayedShareFetch` is updated to use current `readAsync`. Post that we could refactor log reader - I would recommend 2 methods > > `read(options)` which always fetches remote and `read(options, boolean fetchRemote)` for flexibility, both returning completable futures. The `readRemote` method in the impl class is private. > > Post merge of this PR, and updating `DelayedShareFetch`, we will raise a minor PR to update `LogReader` with aforementioned API. Sounds good but why not to just have `read(options, boolean fetchRemote)` method alone. So DelayedShareFetch will use `false` as `fetchRemote` for now and DLQ will usa as true. Later I ll integrate the DelayedShareFetch correctly? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
