[GitHub] [flink] PatrickRen commented on pull request #13606: [FLINK-19554][e2e] Implement a unified testing framework for connectors

[GitBox <gi...@apache.org>]

PatrickRen commented on pull request #13606:
URL: https://github.com/apache/flink/pull/13606#issuecomment-718051348


   Hi @becketqin ,
   
   Thank you very much for your review~ I refactored the testing framework based on your comments, and I added a README file for describing the top-level design of the new framework. 
   
   About those comments you left above, I fixed most of them except the hard-coded host and port of the ControllableSource. I think it's necessary to keep the port number a fixed value under some scenarios. For example if I'm using ControllableSource in Testcontainers, I have to know exactly what the port number is so that I can expose this port out of the container. But yes your concern about conflict is indeed reasonable. Do you have any suggestions of solving this? 
   
   Another problem is the incompatibility between different Flink versions (specifically 1.11.2 and current 1.12-SNAPSHOT on master). Docker repository only has our released versions of images like 1.11.2. If I use ```StreamExecutionEnvironment.execute()``` in 1.12-SNAPSHOT to submit the job to 1.11.2 cluster on Docker, the job will be failed because of an NPE from the operator. So currently the ```KafkaE2ETest``` will fail because of this incompatibility. Although I can use a 1.11.2 ```StreamExecutionEnvironment``` to make the case work, it'll be weird always using a version behind the latest one. 
   
   Looking forward to your advices~


----------------------------------------------------------------
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:
users@infra.apache.org