Dennis-Mircea commented on code in PR #1111:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/1111#discussion_r3218009256


##########
examples/flink-python-example/README.md:
##########
@@ -63,7 +63,10 @@ Dockerfile
 
 **Step 2**: Build docker image
 
-Check this 
[doc](https://nightlies.apache.org/flink/flink-docs-master/docs/deployment/resource-providers/standalone/docker/#using-flink-python-on-docker)
 for more details about building Pyflink image. Note, Pyflink 1.15.3 is only 
supported on x86 arch.  
+Check this 
[doc](https://nightlies.apache.org/flink/flink-docs-master/docs/deployment/resource-providers/standalone/docker/#using-flink-python-on-docker)
 for more details about building a PyFlink image. The included Dockerfile is 
based on `flink:2.2` and installs PyFlink 2.2.0 (which supports Python 3.9, 
3.10, 3.11 and 3.12).
+
+Note: PyFlink's `pemja` dependency only ships pre-built Linux wheels for 
`x86_64`. On other architectures (e.g. Apple Silicon) the build falls back to 
compiling from source, which fails because `flink:2.2` ships a JRE-only image. 
Build for `linux/amd64` explicitly in that case (`docker build --platform 
linux/amd64 ...`).

Review Comment:
   Nice call out about `pemja` and `--platform linux/amd64`, but the code block 
right below still shows the plain `docker build` command without any concrete 
variant for non-x86 arch. I'd suggest phrasing it this way:
   
   ```suggestion
   Note: PyFlink's `pemja` dependency only ships pre-built Linux wheels for 
`x86_64`. On other architectures (e.g. Apple Silicon) the build falls back to 
compiling from source, which fails because `flink:2.2` ships a JRE-only image, 
so on those hosts you must pass `--platform linux/amd64` to `docker build`.
   
   ```bash
   # Uncomment when building for local minikube env:
   # eval $(minikube docker-env)
   
   # Uncomment on Apple Silicon / non-x86 hosts (pemja has no aarch64 wheel):
   # DOCKER_BUILDKIT=1 docker build --platform linux/amd64 . -t 
flink-python-example:latest
   
   # Comment out on Apple Silicon / non-x86 hosts:
   DOCKER_BUILDKIT=1 docker build . -t flink-python-example:latest
   ```
   ```



##########
examples/flink-python-example/README.md:
##########
@@ -46,11 +46,11 @@ user-defined arguments should be placed in the end. Check 
the [doc](https://nigh
 
 A working example would be:
 ```yaml
-args: ["-pyfs", "/opt/flink/usrlib/pythonjob/python_demo.py", "-pyclientexec", 
"/usr/local/bin/python3", "-py", "/opt/flink/usrlib/pythonjob/python_demo.py", 
"-myarg", "123"]
+args: ["-pyfs", "/opt/flink/usrlib/pythonjob/python_demo.py", "-pyclientexec", 
"/usr/bin/python3", "-py", "/opt/flink/usrlib/pythonjob/python_demo.py", 
"-myarg", "123"]

Review Comment:
   While we're refreshing this example, can we also fix a pre‑existing 
inconsistency here? These two snippets reference 
`/opt/flink/usrlib/pythonjob/python_demo.py` (with a `pythonjob/` subdir), but 
both the Dockerfile (`ADD python_demo.py /opt/flink/usrlib/python_demo.py`) and 
`python-example.yaml` (`-py /opt/flink/usrlib/python_demo.py`) place the script 
directly under `/opt/flink/usrlib/`. Users following the README verbatim will 
get a `FileNotFoundException`. Easiest fix is to drop the `pythonjob/` segment 
from both args examples so all three files agree.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to