You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "dannycranmer (via GitHub)" <gi...@apache.org> on 2023/12/18 16:22:19 UTC

[PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

dannycranmer opened a new pull request, #121:
URL: https://github.com/apache/flink-connector-aws/pull/121

   ## Purpose of the change
   
   Inspired by https://github.com/apache/flink-connector-kafka/pull/69, moves `kinesis.py` and `test_kinesis.py` from `apache/flink` to `apache/flink-connector-aws`. 
   
   When running `mvn clean install` downloads the testing infra scripts from the main Flink repo (flink-python/dev/build-wheels.sh, flink-python/dev/install_command.sh, flink-python/dev/lint-python.sh)
   
   ## Verifying this change
   
   Tests can be run by the following commands:
   
   ```
   cd flink-python-connector-aws
   chmod a+x dev/* 
   ./dev/lint-python.sh -e mypy,sphinx
   ```
   
   Additionally I have build the archive via `./dev/build-wheels.sh` and then included in a local sample app that consumes from Kinesis and prints to terminal.
   
   ## Significant changes
   *(Please check any boxes [x] if the answer is "yes". You can first publish the PR and check them afterwards, for convenience.)*
   - [ ] Dependencies have been added or upgraded
   - [ ] Public API has been changed (Public API is any class annotated with `@Public(Evolving)`)
   - [ ] Serializers have been changed
   - [ ] New feature has been introduced
     - If yes, how is this documented? (not applicable / docs / JavaDocs / not documented)
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "hlteoh37 (via GitHub)" <gi...@apache.org>.
hlteoh37 commented on PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#issuecomment-1873879674

   LGTM - only remaining comment is from @z3d1k above 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "hlteoh37 (via GitHub)" <gi...@apache.org>.
hlteoh37 commented on PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#issuecomment-1864482764

   Seems like CI is failing due to license issues. Is there a reason we need the junit jar in our packaged connector? Maybe we can just exclude
   
   > 16:42:35,527 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] ERROR org.apache.flink.tools.ci.licensecheck.JarFileChecker        [] - Missing META-INF/NOTICE in /home/runner/work/flink-connector-aws/flink-connector-aws/flink-connector-aws/flink-python-connector-aws/target/test-dependencies/junit-4.13.2.jar
   16:42:35,527 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] ERROR org.apache.flink.tools.ci.licensecheck.JarFileChecker        [] - Missing META-INF/LICENSE in /home/runner/work/flink-connector-aws/flink-connector-aws/flink-connector-aws/flink-python-connector-aws/target/test-dependencies/junit-4.13.2.jar
   16:42:35,529 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] ERROR org.apache.flink.tools.ci.licensecheck.JarFileChecker        [] - Jar file /home/runner/work/flink-connector-aws/flink-connector-aws/flink-connector-aws/flink-python-connector-aws/target/test-dependencies/junit-4.13.2.jar contains a LICENSE file in an unexpected location: /LICENSE-junit.txt
   16:42:46,840 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] WARN  org.apache.flink.tools.ci.licensecheck.LicenseChecker        [] - Found a total of 3 severe license issues
   
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "dannycranmer (via GitHub)" <gi...@apache.org>.
dannycranmer commented on PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#issuecomment-1867566375

   > Seems like CI is failing due to license issues. Is there a reason we need the junit jar in our packaged connector? Maybe we can just exclude
   > 
   > > 16:42:35,527 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] ERROR org.apache.flink.tools.ci.licensecheck.JarFileChecker        [] - Missing META-INF/NOTICE in /home/runner/work/flink-connector-aws/flink-connector-aws/flink-connector-aws/flink-python-connector-aws/target/test-dependencies/junit-4.13.2.jar
   > > 16:42:35,527 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] ERROR org.apache.flink.tools.ci.licensecheck.JarFileChecker        [] - Missing META-INF/LICENSE in /home/runner/work/flink-connector-aws/flink-connector-aws/flink-connector-aws/flink-python-connector-aws/target/test-dependencies/junit-4.13.2.jar
   > > 16:42:35,529 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] ERROR org.apache.flink.tools.ci.licensecheck.JarFileChecker        [] - Jar file /home/runner/work/flink-connector-aws/flink-connector-aws/flink-connector-aws/flink-python-connector-aws/target/test-dependencies/junit-4.13.2.jar contains a LICENSE file in an unexpected location: /LICENSE-junit.txt
   > > 16:42:46,840 [org.apache.flink.tools.ci.licensecheck.LicenseChecker.main()] WARN  org.apache.flink.tools.ci.licensecheck.LicenseChecker        [] - Found a total of 3 severe license issues
   
   Fixed


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "dannycranmer (via GitHub)" <gi...@apache.org>.
dannycranmer merged PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "dannycranmer (via GitHub)" <gi...@apache.org>.
dannycranmer commented on code in PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#discussion_r1434863721


##########
flink-connector-aws/flink-python-connector-aws/pom.xml:
##########
@@ -0,0 +1,223 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.flink</groupId>
+        <artifactId>flink-connector-aws-parent</artifactId>
+        <version>4.3-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>flink-python-connector-aws</artifactId>
+    <name>Flink : Connectors : AWS : Python</name>
+
+    <packaging>pom</packaging>
+
+    <properties>
+        <python.infra.download.skip>false</python.infra.download.skip>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-kinesis</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-aws-kinesis-firehose</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-runtime</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-streaming-java</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-connector-test-utils</artifactId>
+            <version>${flink.version}</version>
+        </dependency>

Review Comment:
   The packaging of this project is `pom`, so it is really a no-op changing to provided. However, will do this so it reads better



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "hlteoh37 (via GitHub)" <gi...@apache.org>.
hlteoh37 commented on code in PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#discussion_r1432721109


##########
.github/workflows/nightly.yml:
##########
@@ -31,4 +31,12 @@ jobs:
       flink_version: ${{ matrix.flink }}
       flink_url: https://s3.amazonaws.com/flink-nightly/flink-${{ matrix.flink }}-bin-scala_2.12.tgz
       cache_flink_binary: false
-    secrets: inherit
\ No newline at end of file
+    secrets: inherit
+
+  python_test:
+    strategy:
+      matrix:

Review Comment:
   Wonder if we should consider using a similar matrix for python versions



##########
flink-connector-aws/flink-python-connector-aws/pom.xml:
##########
@@ -0,0 +1,223 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.flink</groupId>
+        <artifactId>flink-connector-aws-parent</artifactId>
+        <version>4.3-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>flink-python-connector-aws</artifactId>
+    <name>Flink : Connectors : AWS : Python</name>
+
+    <packaging>pom</packaging>
+
+    <properties>
+        <python.infra.download.skip>false</python.infra.download.skip>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-kinesis</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-aws-kinesis-firehose</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-runtime</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-streaming-java</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-connector-test-utils</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-antrun-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>clean</id>
+                        <phase>clean</phase>
+                        <goals>
+                            <goal>run</goal>
+                        </goals>
+                        <configuration>
+                            <target>
+                                <delete dir="${project.basedir}/.tox"/>
+                                <delete dir="${project.basedir}/apache_flink_connectors_kafka.egg-info"/>

Review Comment:
   This doesn't seem right for kinesis



##########
.gitignore:
##########
@@ -36,4 +36,20 @@ tools/flink
 tools/flink-*
 tools/releasing/release
 tools/japicmp-output
-*/.idea/
\ No newline at end of file
+*/.idea/
+
+# Generated file, do not store in git

Review Comment:
   Should we remove this comment if we plan to store this?



##########
flink-connector-aws/flink-python-connector-aws/pom.xml:
##########
@@ -0,0 +1,223 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.flink</groupId>
+        <artifactId>flink-connector-aws-parent</artifactId>
+        <version>4.3-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>flink-python-connector-aws</artifactId>
+    <name>Flink : Connectors : AWS : Python</name>
+
+    <packaging>pom</packaging>
+
+    <properties>
+        <python.infra.download.skip>false</python.infra.download.skip>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-kinesis</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-aws-kinesis-firehose</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-runtime</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-streaming-java</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-connector-test-utils</artifactId>
+            <version>${flink.version}</version>
+        </dependency>

Review Comment:
   How come we need this at `compile` level?



##########
flink-connector-aws/flink-python-connector-aws/pom.xml:
##########
@@ -0,0 +1,223 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.flink</groupId>
+        <artifactId>flink-connector-aws-parent</artifactId>
+        <version>4.3-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>flink-python-connector-aws</artifactId>
+    <name>Flink : Connectors : AWS : Python</name>
+
+    <packaging>pom</packaging>
+
+    <properties>
+        <python.infra.download.skip>false</python.infra.download.skip>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-kinesis</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-aws-kinesis-firehose</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-runtime</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-streaming-java</artifactId>
+            <version>${flink.version}</version>
+        </dependency>

Review Comment:
   Why do we need these at `compile` level?



##########
flink-connector-aws/flink-python-connector-aws/pom.xml:
##########
@@ -0,0 +1,223 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.flink</groupId>
+        <artifactId>flink-connector-aws-parent</artifactId>
+        <version>4.3-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>flink-python-connector-aws</artifactId>
+    <name>Flink : Connectors : AWS : Python</name>
+
+    <packaging>pom</packaging>
+
+    <properties>
+        <python.infra.download.skip>false</python.infra.download.skip>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-kinesis</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-aws-kinesis-firehose</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-runtime</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-streaming-java</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-connector-test-utils</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-antrun-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>clean</id>
+                        <phase>clean</phase>
+                        <goals>
+                            <goal>run</goal>
+                        </goals>
+                        <configuration>
+                            <target>
+                                <delete dir="${project.basedir}/.tox"/>
+                                <delete dir="${project.basedir}/apache_flink_connectors_kafka.egg-info"/>
+                                <delete dir="${project.basedir}/dev/.conda"/>
+                                <delete dir="${project.basedir}/dev/download"/>
+                                <delete dir="${project.basedir}/dev/log"/>
+                                <delete dir="${project.basedir}/build"/>
+                                <delete dir="${project.basedir}/dist"/>
+                                <delete dir="${project.basedir}/pyflink/lib"/>
+                                <delete file="${project.basedir}/dev/.stage.txt"/>
+                                <delete file="${project.basedir}/dev/install_command.sh"/>
+                                <delete file="${project.basedir}/dev/lint-python.sh"/>
+                                <delete file="${project.basedir}/dev/build-wheels.sh"/>
+                                <delete file="${project.basedir}/dev/glibc_version_fix.h"/>
+                                <delete file="${project.basedir}/dev/dev-requirements.txt"/>
+                                <delete file="${project.basedir}/pyflink/datastream/connectors/kafka_connector_version.py"/>

Review Comment:
   This doesn't seem right for kinesis!



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "dannycranmer (via GitHub)" <gi...@apache.org>.
dannycranmer commented on code in PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#discussion_r1434864490


##########
flink-connector-aws/flink-python-connector-aws/pom.xml:
##########
@@ -0,0 +1,223 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.flink</groupId>
+        <artifactId>flink-connector-aws-parent</artifactId>
+        <version>4.3-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>flink-python-connector-aws</artifactId>
+    <name>Flink : Connectors : AWS : Python</name>
+
+    <packaging>pom</packaging>
+
+    <properties>
+        <python.infra.download.skip>false</python.infra.download.skip>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-kinesis</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-aws-kinesis-firehose</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-runtime</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-streaming-java</artifactId>
+            <version>${flink.version}</version>
+        </dependency>

Review Comment:
   The packaging of this project is pom, so it is really a no-op changing to provided. However, will do this so it reads better. 
   
   The dependencies are copied into a test-dependencies folder, by the copy plugin, for use in the tests



##########
flink-connector-aws/flink-python-connector-aws/pom.xml:
##########
@@ -0,0 +1,223 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache.flink</groupId>
+        <artifactId>flink-connector-aws-parent</artifactId>
+        <version>4.3-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>flink-python-connector-aws</artifactId>
+    <name>Flink : Connectors : AWS : Python</name>
+
+    <packaging>pom</packaging>
+
+    <properties>
+        <python.infra.download.skip>false</python.infra.download.skip>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-kinesis</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-sql-connector-aws-kinesis-firehose</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-runtime</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-streaming-java</artifactId>
+            <version>${flink.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.flink</groupId>
+            <artifactId>flink-connector-test-utils</artifactId>
+            <version>${flink.version}</version>
+        </dependency>

Review Comment:
   The dependencies are copied into a test-dependencies folder, by the copy plugin, for use in the tests



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "dannycranmer (via GitHub)" <gi...@apache.org>.
dannycranmer commented on code in PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#discussion_r1434873885


##########
.github/workflows/nightly.yml:
##########
@@ -31,4 +31,12 @@ jobs:
       flink_version: ${{ matrix.flink }}
       flink_url: https://s3.amazonaws.com/flink-nightly/flink-${{ matrix.flink }}-bin-scala_2.12.tgz
       cache_flink_binary: false
-    secrets: inherit
\ No newline at end of file
+    secrets: inherit
+
+  python_test:
+    strategy:
+      matrix:

Review Comment:
   The python matrix is handled within the step. This is a good idea, but will require a change to the `flink-connector-shared-utils`, will create a follow up Jira



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33560][Connectors/AWS] Externalize AWS Python connectors from Flink to AWS project [flink-connector-aws]

Posted by "z3d1k (via GitHub)" <gi...@apache.org>.
z3d1k commented on code in PR #121:
URL: https://github.com/apache/flink-connector-aws/pull/121#discussion_r1439274934


##########
flink-python/setup.py:
##########
@@ -0,0 +1,153 @@
+################################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+from __future__ import print_function
+
+import io
+import os
+import sys
+
+from setuptools import setup
+from shutil import rmtree
+from xml.etree import ElementTree as ET
+
+PACKAGE_NAME = 'apache-flink-connector-aws'
+# Source files, directories
+CURRENT_DIR = os.path.abspath(os.path.dirname(__file__))
+POM_FILE = os.path.join(CURRENT_DIR, '../pom.xml')
+README_FILE = os.path.join(CURRENT_DIR, 'README.txt')
+
+# Generated files and directories
+VERSION_FILE = os.path.join(CURRENT_DIR, 'pyflink/datastream/connectors/aws_connector_version.py')
+DEPENDENCY_FILE = os.path.join(CURRENT_DIR, 'dev/dev-requirements.txt')
+
+
+# Removes a file or directory if exists.
+def remove_if_exists(file_path):
+    if os.path.exists(file_path):
+        if os.path.isfile(file_path):
+            os.remove(file_path)
+        if os.path.isdir(file_path):
+            rmtree(file_path)
+
+
+# Reads the content of the README.txt file.
+def readme_content():
+    with io.open(README_FILE, 'r', encoding='utf-8') as f:
+        return f.read()
+
+
+# Reads the parameters used by the setup command.
+# The source is the aws_connector_version.py and the README.txt.
+def setup_parameters():
+    try:
+        exec(open(VERSION_FILE).read())
+        return locals()['__connector_version__'], locals()['__flink_dependency__'], readme_content()
+    except IOError:
+        print("Failed to load PyFlink version file for packaging. " +
+              "'%s' not found!" % VERSION_FILE,
+              file=sys.stderr)
+        sys.exit(-1)
+
+
+# Reads and parses the flink-connector-aws main pom.xml.
+# Based on the version data in the pom.xml prepares the pyflink dir:
+#  - Generates aws_connector_version.py
+#  - Generates dev-requirements.txt
+#  - Copies the flink-sql-connector-kinesis.jar to the pyflink/lib dir
+def prepare_pyflink_dir():
+    # source files
+    pom_root = ET.parse(POM_FILE).getroot()
+    flink_version = pom_root.findall(
+        "./{http://maven.apache.org/POM/4.0.0}properties/" +
+        "{http://maven.apache.org/POM/4.0.0}flink.version"
+    )[0].text
+    connector_version = pom_root.findall(
+        "./{http://maven.apache.org/POM/4.0.0}version")[0].text.replace("-SNAPSHOT", ".dev0")
+
+    flink_dependency = "apache-flink>=" + flink_version
+
+    with io.open(VERSION_FILE, 'w', encoding='utf-8') as f:
+        f.write('# Generated file, do not edit\n')
+        f.write('__connector_version__ = "' + connector_version + '"\n')
+        f.write('__flink_dependency__ = "' + flink_dependency + '"\n')
+
+    with io.open(DEPENDENCY_FILE, 'w', encoding='utf-8') as f:
+        f.write('# Generated file, do not edit\n')
+        f.write(flink_dependency + '\n')
+
+# Main
+print("Python version used to package: " + sys.version)
+
+# Python version check
+if sys.version_info < (3, 7):
+    print("Python versions prior to 3.7 are not supported for PyFlink.",
+          file=sys.stderr)
+    sys.exit(-1)
+
+# Checks the running environment:
+#  - In the connector source root directory - package preparation
+#  - Otherwise - package deployment
+in_flink_source = os.path.isfile(
+    "../flink-connector-aws/flink-connector-kinesis/src/main" +
+    "/java/org/apache/flink/streaming/connectors/kinesis/FlinkKinesisConsumer.java")
+
+# Cleans up the generated files and directories and regenerate them.
+if in_flink_source:
+    remove_if_exists(VERSION_FILE)
+    remove_if_exists(DEPENDENCY_FILE)
+    prepare_pyflink_dir()
+    print("\nPreparing Flink AWS connector package")
+
+# Reads the current setup data from the aws_connector_version.py file and the README.txt
+(connector_version, flink_dependency, long_description) = setup_parameters()
+
+print("\nConnector version: " + connector_version)
+print("Flink dependency: " + flink_dependency + "\n")
+
+if in_flink_source:
+    # Removes temporary directory used by the setup tool
+    remove_if_exists(PACKAGE_NAME.replace('-', '_') + '.egg-info')
+
+# Runs the python setup
+setup(
+    name=PACKAGE_NAME,
+    version=connector_version,
+    include_package_data=True,
+    url='https://flink.apache.org',
+    license='https://www.apache.org/licenses/LICENSE-2.0',
+    author='Apache Software Foundation',
+    author_email='dev@flink.apache.org',
+    python_requires='>=3.8',

Review Comment:
   This seems to contradict information in other places in this file - everywhere else current minimal Python version is set to 3.7.
   Not sure if this can cause any issues.
   `flink-python/tox.ini` also use 3.8 as a minimum version.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org