Hi Peter, Thanks for picking this up! Please find my thoughts inline.
BR, G On Mon, Nov 27, 2023 at 4:05 PM Péter Váry <peter.vary.apa...@gmail.com> wrote: > Hi Team, > > During some, mostly unrelated work we come to realize that during the > externalization of the connector's python modules and the related tests are > not moved to the respective connectors repository. > We created the jiras [1] to create the infra, and to move the python code > and the tests to the respective repos. > > When working on this code, I have found several oddities, which I would > like to hear the community's opinion on: - Does anyone know what the > site-packages/pyflink/opt/flink-sql-client-1.18.0.jar supposed to do? We > specifically put it there [2], but then we ignore it when we check the > directory of jars [3]. If there is no objection, I would remove it. > +1 on removing that unless there are objections. As I see the jar is not included in the classpath so not used. > - I would like to use the `opt` directory, to contain optional jars created > by the connectors, like flink-sql-connector-kafka-3.1.x.jar. > +1 on that. The other options would be: * plugins -> Kafka connector is not used as plugin w/ separate classloader so this solution is not preferred * lib -> here we store our core jars which are part of the main Flink repo so this solution is not preferred All in all I'm fine to use opt. > Also, the lint-python.sh [4], and install_command.sh [5] provides the base > of the testing infra. Would there be any objections to mark these as a > public apis for the connectors? I fully agree that we should avoid code duplications and re-using the existing code parts but making them API is not what I can imagine. The reason behind is simple. That would just block development in mid/long term in Flink main repo. * lint-python.sh is far from a stable version from my understanding + that way Flink will contain the superset of maybe 10+ connector needs * install_command.sh contains generic and Flink specific parts. When we extract the Flink specific parts we can declare it logically API What I can imagine is to make the mentioned scripts more configurable and make them @Experimental. When a development team of a connector is not happy to use it as-is then a deep-copy and/or rewrite is still an option. > Thanks, > Peter > > [1] - https://issues.apache.org/jira/browse/FLINK-33528 > [2] - > > https://github.com/apache/flink/blob/2da9a9639216b8c48850ee714065f090a80dcd65/flink-python/apache-flink-libraries/setup.py#L129-L130 > [3] - > > https://github.com/apache/flink/blob/2da9a9639216b8c48850ee714065f090a80dcd65/flink-python/pyflink/pyflink_gateway_server.py#L183-L190 > [4] - > https://github.com/apache/flink/blob/master/flink-python/dev/lint-python.sh > [5] - > > https://github.com/apache/flink/blob/master/flink-python/dev/install_command.sh >
