zsxwing commented on pull request #31219: URL: https://github.com/apache/spark/pull/31219#issuecomment-764870906
I'm happy to see you plan to build a new state store backend. But I don't agree we should merge refactoring first without posting any concrete plan. > * The community did a lot of refactoring, either main or test code. So the first point sounds not make sense. Doing refactoring like this PR increases the backporting cost. So I would like to see the value of this first. > * Moved tests are tests for general StateStore behavior. They should be put in a general test suite, not HDFS StateStore-specific tests. Specifically, StateStore is an API that third-party uses for other StateStore implementation. For now. when writing specific StateStore test suite, it needs to duplicate these general tests, otherwise it lacks of test coverage. I don't think this is a good pattern regarding an API like that. Test codes are not public APIs. They are changed frequently. I would recommend a third-party StateStore implementation to build their own tests. > 3\. So I also don't think the second point make sense. There is no test specific to so called internal information. They are all based on the StateStore behavior. I meant the information that you plan to build a new state store backend. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
