jsancio commented on PR #12414: URL: https://github.com/apache/kafka/pull/12414#issuecomment-1206405757
> Thanks @jsancio, this is helpful. I'll go ahead and make these changes! @ashmeet13 , I thought about this some more and I don't think we should include the reason for the snapshot in the `raft` module and types. In other words, I think we should: 1. Move the `SnapshotReason` type from `raft/src/main/java/org/apache/kafka/raft` to `metadata/src/main/java/org/apache/kafka/metadata/util`. 2. Keep the old signature for the createNewSnapshot. I suspect that we can undo all of the changes you made to the `raft` module. I think this now because the reasons for creating a snapshot are specific to `metadata` and not `raft`. We have an interest in using the `raft` module and the KRaft protocol to solve other problems. I don't think we will have the same reasons for wanting to generate snapshot in those use cases. We should still use the `SnapshotReason` type and I am just concern on where it should live. Thanks and excuse my earlier misguided advice. -- 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
