You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "apoorvmittal10 (via GitHub)" <gi...@apache.org> on 2024/09/26 18:40:34 UTC
[PR] KAFKA-17626: Move common fetch related classes in storage to storage API [kafka]
apoorvmittal10 opened a new pull request, #17289:
URL: https://github.com/apache/kafka/pull/17289
Moved classes which are common in fetch and exposed to share fetch in storage-api module.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17626: Move common fetch related classes from storage to storage API [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #17289:
URL: https://github.com/apache/kafka/pull/17289#discussion_r1777597030
##########
checkstyle/import-control-share.xml:
##########
@@ -45,7 +45,7 @@
<allow pkg="org.apache.kafka.server.share" />
<subpackage name="fetch">
- <allow class="org.apache.kafka.storage.internals.log.FetchParams"/>
Review Comment:
Can you check other import control files to see if any others need cleanup?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17626: Move common fetch related classes from storage to server-common [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #17289:
URL: https://github.com/apache/kafka/pull/17289#issuecomment-2378178990
@apoorvmittal10 can you update the PR description?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17626: Move common fetch related classes from storage to server-common [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #17289:
URL: https://github.com/apache/kafka/pull/17289#discussion_r1777696462
##########
checkstyle/import-control-share.xml:
##########
@@ -45,7 +45,7 @@
<allow pkg="org.apache.kafka.server.share" />
<subpackage name="fetch">
- <allow class="org.apache.kafka.storage.internals.log.FetchParams"/>
Review Comment:
I checked elsewhere but can't find anything to clean.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17626: Move common fetch related classes from storage to storage API [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #17289:
URL: https://github.com/apache/kafka/pull/17289#issuecomment-2377747380
@chia7712 I wasn't aware that storage-api was meant as a public API. I thought this module was just decoupling internal dependencies. Thanks for the correction 😅 (this change was my suggestion in a previous PR).
@apoorvmittal10 Seems like server-common is a better place after all.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17626: Move common fetch related classes from storage to server-common [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah merged PR #17289:
URL: https://github.com/apache/kafka/pull/17289
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17626: Move common fetch related classes from storage to storage API [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #17289:
URL: https://github.com/apache/kafka/pull/17289#issuecomment-2377736825
Apologies, but I assume the code in storage-api is intended to serve as a plugin interface. For instance, the current storage-api code provides a pluggable API for remote storage. Similarly, tool-api offers a plugin interface for console tools, and group-coordinator-api is designed for server partition assignors.
Do the common fetch-related classes serve a similar purpose? Or is the main goal simply to move them into a "common" module? If that's the case, perhaps we could relocate them to server-common?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17626: Move common fetch related classes from storage to server-common [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #17289:
URL: https://github.com/apache/kafka/pull/17289#issuecomment-2378103067
@mumrah @chia7712 Thanks for the review. The build is green.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org