Copilot commented on code in PR #13215: URL: https://github.com/apache/gluten/pull/13215#discussion_r4260070853
########## .github/workflows/util/pyspark_arrow_test.py: ########## @@ -0,0 +1,42 @@ +# 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. + +# Runs PySpark's Arrow conversions (createDataFrame(pandas_df) and toPandas()) with the Gluten +# bundle on the classpath, to check that Spark's own Arrow code still works next to the Arrow +# classes Gluten bundles. Submit it with the bundle in spark.{driver,executor}.extraClassPath and +# spark.plugins=org.apache.gluten.GlutenPlugin, see the pyspark-arrow-test job in +# velox_backend_x86.yml. + +import numpy as np +import pandas as pd +from pyspark.sql import SparkSession + +spark = SparkSession.builder.appName("pyspark-arrow-test").getOrCreate() +assert "org.apache.gluten.GlutenPlugin" in spark.conf.get("spark.plugins", ""), \ + "Gluten is not enabled, submit with spark.plugins=org.apache.gluten.GlutenPlugin" Review Comment: Using `assert` for runtime validation is fragile because asserts can be disabled with Python optimization flags (`-O`), which would make this test silently pass even if Gluten/Arrow behavior is broken. Prefer explicit checks that raise `AssertionError` (or `RuntimeError`) unconditionally so the CI job always fails on unexpected behavior. ########## .github/workflows/util/pyspark_arrow_test.py: ########## @@ -0,0 +1,42 @@ +# 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. + +# Runs PySpark's Arrow conversions (createDataFrame(pandas_df) and toPandas()) with the Gluten +# bundle on the classpath, to check that Spark's own Arrow code still works next to the Arrow +# classes Gluten bundles. Submit it with the bundle in spark.{driver,executor}.extraClassPath and +# spark.plugins=org.apache.gluten.GlutenPlugin, see the pyspark-arrow-test job in +# velox_backend_x86.yml. + +import numpy as np +import pandas as pd +from pyspark.sql import SparkSession + +spark = SparkSession.builder.appName("pyspark-arrow-test").getOrCreate() +assert "org.apache.gluten.GlutenPlugin" in spark.conf.get("spark.plugins", ""), \ + "Gluten is not enabled, submit with spark.plugins=org.apache.gluten.GlutenPlugin" + +spark.conf.set("spark.sql.execution.arrow.pyspark.enabled", "true") +spark.conf.set("spark.sql.execution.arrow.pyspark.fallback.enabled", "false") + +pdf = pd.DataFrame(np.full((100, 3), 5)) + +df = spark.createDataFrame(pdf) +result_pdf = df.select("*").toPandas() +print("Pandas DataFrame result statics:\n%s\n" % str(result_pdf.describe())) Review Comment: Typo in log message: 'statics' should be 'statistics'. ########## .github/workflows/util/pyspark_arrow_test.py: ########## @@ -0,0 +1,42 @@ +# 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. + +# Runs PySpark's Arrow conversions (createDataFrame(pandas_df) and toPandas()) with the Gluten +# bundle on the classpath, to check that Spark's own Arrow code still works next to the Arrow +# classes Gluten bundles. Submit it with the bundle in spark.{driver,executor}.extraClassPath and +# spark.plugins=org.apache.gluten.GlutenPlugin, see the pyspark-arrow-test job in +# velox_backend_x86.yml. + +import numpy as np +import pandas as pd +from pyspark.sql import SparkSession + +spark = SparkSession.builder.appName("pyspark-arrow-test").getOrCreate() +assert "org.apache.gluten.GlutenPlugin" in spark.conf.get("spark.plugins", ""), \ + "Gluten is not enabled, submit with spark.plugins=org.apache.gluten.GlutenPlugin" + +spark.conf.set("spark.sql.execution.arrow.pyspark.enabled", "true") +spark.conf.set("spark.sql.execution.arrow.pyspark.fallback.enabled", "false") + +pdf = pd.DataFrame(np.full((100, 3), 5)) + +df = spark.createDataFrame(pdf) +result_pdf = df.select("*").toPandas() +print("Pandas DataFrame result statics:\n%s\n" % str(result_pdf.describe())) +assert result_pdf.shape == (100, 3), "Unexpected shape %s" % str(result_pdf.shape) +assert (result_pdf.to_numpy() == 5).all(), "Unexpected values in the result" Review Comment: Using `assert` for runtime validation is fragile because asserts can be disabled with Python optimization flags (`-O`), which would make this test silently pass even if Gluten/Arrow behavior is broken. Prefer explicit checks that raise `AssertionError` (or `RuntimeError`) unconditionally so the CI job always fails on unexpected behavior. ########## .github/workflows/velox_backend_x86.yml: ########## @@ -927,6 +927,48 @@ jobs: name: spark-test-spark35-group${{ matrix.group }}-golden-files path: /tmp/tpch-approved-plan/** + pyspark-arrow-test-spark35: + # PySpark's Arrow conversions run Spark's own Arrow code, which breaks if the Arrow classes + # in the Gluten bundle no longer link with it (e.g. the NoSuchMethodError fixed by #13096). + needs: [detect-changes, build-native-lib-centos-8] + if: >- + needs.detect-changes.outputs.java == 'true' || + needs.detect-changes.outputs.shims35 == 'true' || + needs.detect-changes.outputs.cpp == 'true' + runs-on: ubuntu-22.04 + container: apache/gluten:centos-8-jdk8 + steps: + - uses: actions/checkout@v7 + - name: Download All Artifacts + uses: actions/download-artifact@v8 + with: + name: velox-native-lib-centos-8-${{github.sha}} + path: ./cpp/build/releases/ + - name: Prepare + run: | + dnf module -y install python39 && \ + alternatives --set python3 /usr/bin/python3.9 && \ + pip3 install setuptools==77.0.3 && \ + pip3 install pyspark==3.5.5 pandas==2.2.3 pyarrow==20.0.0 + - name: Build Gluten bundle + run: | + cd $GITHUB_WORKSPACE/ + $MVN_CMD clean package -Pspark-3.5 -Pjava-8 -Pbackends-velox -DskipTests + - name: Run PySpark Arrow conversions with the Gluten bundle + run: | + cd $GITHUB_WORKSPACE/ + # The pip pyspark package is a full Spark 3.5.5 distribution. + export SPARK_HOME=$(python3 -c 'import os, pyspark; print(os.path.dirname(pyspark.__file__))') + BUNDLE=$(ls package/target/gluten-velox-bundle-spark3.5_2.12-*.jar) + $SPARK_HOME/bin/spark-submit --master 'local[2]' \ + --conf spark.driver.extraClassPath=$BUNDLE \ + --conf spark.executor.extraClassPath=$BUNDLE \ Review Comment: `BUNDLE=$(ls ...)` will fail if the glob matches 0 files, and will produce an invalid classpath if it matches more than 1 file (newline-separated). Also, the `extraClassPath` values are unquoted, which can break if the resolved path ever contains shell-special characters. Consider selecting exactly one jar deterministically (and erroring with a clear message otherwise) and quoting `$BUNDLE` in the `--conf` arguments. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
