chickenchickenlove commented on PR #21677: URL: https://github.com/apache/kafka/pull/21677#issuecomment-4019111012
@mjsax Sorry for the sudden mention. 🙇♂️ I opened a draft PR and wanted to use it as a starting point for discussion. While working on this, I came up with two possible directions. #1 The first option is to treat the fact that `MockProcessorContext` no longer supports non-logging, non-caching `WindowStore` as a regression. Because of the changes introduced in #16906, a path that previously appears to have worked with only `StateStoreContext` now requires `InternalProcessorContext`, which means it can no longer be tested with `MockProcessorContext`. In that case, it throws the following error: "This component requires internal features of Kafka Streams and must be disabled for unit tests." However, this path seems to have been testable before, so I think there is a reasonable argument that this should be considered a regression. also, this direction is aligned with what `InMemorySessionStore` already do. #2 The second option is to treat the current behavior as intended and instead adjust `MockProcessorContext#getStateStoreContext()` so that it returns a `StateStoreContext` implementation that also implements `InternalProcessorContext`. With that approach, we could implement only the `InternalProcessorContext` methods that are actually required for non-logging, non-caching `WindowStore` support, and let the other methods throw `UnsupportedOperationException` if they are called. The `StateStoreContext` methods required by `InternalProcessorContext` could simply delegate to the existing `StateStoreContext` implementation. However, in that case, `MockProcessorContext.getStateStoreContext()` would return an `InternalProcessorContext`, which means `asInternalProcessorContext()` would always succeed. Therefore, if `asInternalProcessorContext()` is intended to serve as a guardrail to block certain usage in tests, this approach may not be appropriate, since it would also always pass in paths that are supposed to work in practice. At the moment, the draft PR is based on option #1, but I would really appreciate your thoughts on which direction would be more appropriate! Thanks for taking the time to look at this. 🙇♂️ -- 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]
