syhily commented on pull request #15304: URL: https://github.com/apache/flink/pull/15304#issuecomment-836045335
> Thank you very much for you contribution! The Pulsar connector has been highly anticipated by quite a lot of users, so this PR will have a big impact. > > The design obviously is very close to KafkaSource. It shows that we can probably extract a few building blocks, but that's for the future. For now, it means that the architecture is good and can stay as is. However, please closely check the history of the KafkaSource for any changes that should also be applied here (there were quite a few bugfixes). > > There is a larger chunk of code not covered by tests, especially the very many options to start/stop. I suggest to add some simple unit tests. We have also lots of unused code that either needs to be removed or incorporated. Please also move as many classes as possible away from the "root" package and leave only (or mostly) Public(Evolving) classes there. Reduce visibility of all other classes and annotate all other public classes as Internal. > > Did you actually had the chance to try this connector out against a real Pulsar cluster? How is it performing compared to the old SourceFunction? > > Note: I just reviewed the production code so far. I'll cover the tests with increased coverage. Thank you for your detailed review, I would take over @jianyun8023 job and continue on this PR. Have you finished the review? I would like to fix these review problems. -- 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]
