wangyang0918 commented on pull request #15020: URL: https://github.com/apache/flink/pull/15020#issuecomment-786502426
@SteNicholas Thanks for creating this PR. I have gone though the implementation carefully and want to share some thoughts. Instead of inheritance here, I would suggest to use the composition. Maybe we could refactor this component by the following steps. * Introduce the `PackagedProgramRetrieverAdapter`(maybe other better name), which will provide a builder and select a proper `PackagedProgramRetriever` based on the specified properties. It is just like what you have done in the `ClassPathPackagedProgramRetriever#Builder` * Make `JarFilePackagedProgramRetriever`, `PythonBasedPackagedProgramRetriever`, `ScanClassPathPackagedProgramRetriever`(could be renamed to the new `ClassPathPackagedProgramRetriever`) directly implement the `PackagedProgramRetriever` interface * Maybe we could add the `AbstractPackagedProgramRetriever` if the above three cases have some common member variables or methods to share. After this refactor, we also need to split the tests in `ClassPathPackagedProgramRetrieverTest` to separate files. If the three cases have some common tests, then they could be implemented in the `PackagedProgramRetrieverTestBase`. At last, we should not forget to add some dedicated tests for the `PackagedProgramRetrieverAdapter`. ---------------------------------------------------------------- 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: us...@infra.apache.org
