-
Notifications
You must be signed in to change notification settings - Fork 301
from_protobuf: Re-enable protobuf integration tests on OSS Spark and Databricks [databricks] [fast-ut] [reduced-it] #15814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
50d8373
9b5c992
14fbf8f
9433cd6
807efa4
0054f47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,8 @@ | |||||||||
| # - SPARK_HOME: Path to your Apache Spark installation. | ||||||||||
| # - SKIP_TESTS: If set to true, skips running the Python integration tests. | ||||||||||
| # - INCLUDE_SPARK_AVRO_JAR: If set to true, includes Avro tests. | ||||||||||
| # - INCLUDE_SPARK_PROTOBUF_JAR: Controls external spark-protobuf jar injection; setting it to | ||||||||||
| # false also disables protobuf tests on Apache Spark. | ||||||||||
| # - TEST: Specifies a specific test to run. | ||||||||||
| # - TEST_TAGS: Allows filtering tests based on tags. | ||||||||||
| # - TEST_TYPE: Specifies the type of tests to run. | ||||||||||
|
|
@@ -48,6 +50,9 @@ | |||||||||
| # To run all tests, including Avro tests: | ||||||||||
| # INCLUDE_SPARK_AVRO_JAR=true ./run_pyspark_from_build.sh | ||||||||||
| # | ||||||||||
| # To run without injecting an external spark-protobuf jar: | ||||||||||
| # INCLUDE_SPARK_PROTOBUF_JAR=false ./run_pyspark_from_build.sh | ||||||||||
| # | ||||||||||
| # To run a specific test: | ||||||||||
| # TEST=my_test ./run_pyspark_from_build.sh | ||||||||||
| # | ||||||||||
|
|
@@ -60,6 +65,28 @@ | |||||||||
|
|
||||||||||
| set -ex | ||||||||||
|
|
||||||||||
| is_databricks_runtime_arg() { | ||||||||||
| local expect_runtime_env_value=false | ||||||||||
| local runtime_env="" | ||||||||||
| local arg | ||||||||||
| for arg in "$@"; do | ||||||||||
| if [[ "$expect_runtime_env_value" == "true" ]]; then | ||||||||||
| runtime_env="$arg" | ||||||||||
| expect_runtime_env_value=false | ||||||||||
| continue | ||||||||||
| fi | ||||||||||
| case "$arg" in | ||||||||||
| --runtime_env) | ||||||||||
| expect_runtime_env_value=true | ||||||||||
| ;; | ||||||||||
| --runtime_env=*) | ||||||||||
| runtime_env="${arg#*=}" | ||||||||||
| ;; | ||||||||||
| esac | ||||||||||
| done | ||||||||||
| [[ "${runtime_env,,}" == "databricks" ]] | ||||||||||
| } | ||||||||||
|
|
||||||||||
| SCRIPTPATH="$( cd "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )" | ||||||||||
| cd "$SCRIPTPATH" | ||||||||||
|
|
||||||||||
|
|
@@ -102,6 +129,7 @@ else | |||||||||
| # support alternate local jars NOT building from the source code | ||||||||||
| if [ -d "$LOCAL_JAR_PATH" ]; then | ||||||||||
| AVRO_JARS=$(echo "$LOCAL_JAR_PATH"/spark-avro*.jar) | ||||||||||
| PROTOBUF_JARS=$(echo "$LOCAL_JAR_PATH"/spark-protobuf*.jar) | ||||||||||
| PLUGIN_JAR=$(echo "$LOCAL_JAR_PATH"/rapids-4-spark_*.jar) | ||||||||||
| if [ -f $(echo $LOCAL_JAR_PATH/parquet-hadoop*.jar) ]; then | ||||||||||
| export INCLUDE_PARQUET_HADOOP_TEST_JAR=true | ||||||||||
|
|
@@ -118,6 +146,7 @@ else | |||||||||
| else | ||||||||||
| [[ "$SCALA_VERSION" != "2.12" ]] && TARGET_DIR=${TARGET_DIR/integration_tests/scala$SCALA_VERSION\/integration_tests} | ||||||||||
| AVRO_JARS=$(echo "$TARGET_DIR"/dependency/spark-avro*.jar) | ||||||||||
| PROTOBUF_JARS=$(echo "$TARGET_DIR"/dependency/spark-protobuf*.jar) | ||||||||||
| PARQUET_HADOOP_TESTS=$(echo "$TARGET_DIR"/dependency/parquet-hadoop*.jar) | ||||||||||
| # remove the log4j.properties file so it doesn't conflict with ours, ignore errors | ||||||||||
| # if it isn't present or already removed | ||||||||||
|
|
@@ -143,9 +172,29 @@ else | |||||||||
| AVRO_JARS="" | ||||||||||
| fi | ||||||||||
|
|
||||||||||
| # ALL_JARS includes dist.jar integration-test.jar avro.jar parquet.jar if they exist | ||||||||||
| INCLUDE_SPARK_PROTOBUF_JAR_REQUESTED=$(echo "${INCLUDE_SPARK_PROTOBUF_JAR}" | tr '[:upper:]' '[:lower:]') | ||||||||||
| PROTOBUF_JAR_COUNT=$(readlink -e $PROTOBUF_JARS 2>/dev/null | wc -l) | ||||||||||
| if is_databricks_runtime_arg "$@"; then | ||||||||||
| export INCLUDE_SPARK_PROTOBUF_JAR=false | ||||||||||
| PROTOBUF_JARS="" | ||||||||||
| elif [[ "$INCLUDE_SPARK_PROTOBUF_JAR_REQUESTED" != "false" \ | ||||||||||
| && "$PROTOBUF_JAR_COUNT" -eq 1 ]]; | ||||||||||
| then | ||||||||||
| export INCLUDE_SPARK_PROTOBUF_JAR=true | ||||||||||
| else | ||||||||||
| if [[ "$INCLUDE_SPARK_PROTOBUF_JAR_REQUESTED" != "false" \ | ||||||||||
| && "$PROTOBUF_JAR_COUNT" -gt 1 ]]; then | ||||||||||
| >&2 echo "WARNING: Multiple spark-protobuf jars were found (matched: $PROTOBUF_JARS); not injecting spark-protobuf." | ||||||||||
| elif [[ "$INCLUDE_SPARK_PROTOBUF_JAR_REQUESTED" == "true" ]]; then | ||||||||||
| >&2 echo "WARNING: INCLUDE_SPARK_PROTOBUF_JAR=true was requested but a spark-protobuf jar was not found (searched: $PROTOBUF_JARS)." | ||||||||||
|
Comment on lines
+188
to
+189
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the default configuration and it fails silently. With no jar on disk and
Suggested change
|
||||||||||
| fi | ||||||||||
| export INCLUDE_SPARK_PROTOBUF_JAR=false | ||||||||||
| PROTOBUF_JARS="" | ||||||||||
| fi | ||||||||||
|
|
||||||||||
| # ALL_JARS includes dist.jar integration-test.jar avro.jar parquet.jar protobuf.jar if they exist | ||||||||||
| # Remove non-existing paths and canonicalize the paths including get rid of links and `..` | ||||||||||
| ALL_JARS=$(readlink -e $PLUGIN_JAR $TEST_JARS $AVRO_JARS $PARQUET_HADOOP_TESTS || true) | ||||||||||
| ALL_JARS=$(readlink -e $PLUGIN_JAR $TEST_JARS $AVRO_JARS $PARQUET_HADOOP_TESTS $PROTOBUF_JARS || true) | ||||||||||
| # `:` separated jars | ||||||||||
| ALL_JARS="${ALL_JARS//$'\n'/:}" | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| # Copyright (c) 2026, NVIDIA CORPORATION. | ||
| # | ||
| # Licensed 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. | ||
|
|
||
| import inspect | ||
|
|
||
| import pytest | ||
|
|
||
| from asserts import assert_gpu_fallback_collect | ||
| from marks import allow_non_gpu | ||
| from spark_session import is_spark_protobuf_available | ||
| import pyspark.sql.functions as f | ||
|
|
||
| pytestmark = pytest.mark.skipif( | ||
| not is_spark_protobuf_available(), reason="from_protobuf is unavailable") | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def from_protobuf_fn(): | ||
| from pyspark.sql.protobuf.functions import from_protobuf | ||
| return from_protobuf | ||
|
|
||
|
|
||
| def _encode_varint(value): | ||
| out = bytearray() | ||
| value &= 0xFFFFFFFFFFFFFFFF | ||
| while True: | ||
| bits = value & 0x7F | ||
| value >>= 7 | ||
| if value: | ||
| out.append(bits | 0x80) | ||
| else: | ||
| out.append(bits) | ||
| return bytes(out) | ||
|
|
||
|
|
||
| def _encode_simple_message(i32_value, s_value): | ||
| buf = bytearray() | ||
| buf += _encode_varint((1 << 3) | 0) # field 1, VARINT | ||
| buf += _encode_varint(i32_value) | ||
| s_bytes = s_value.encode("utf-8") | ||
| buf += _encode_varint((2 << 3) | 2) # field 2, LENGTH-DELIMITED | ||
| buf += _encode_varint(len(s_bytes)) | ||
| buf += s_bytes | ||
| return bytes(buf) | ||
|
|
||
|
|
||
| # Avoid depending on whichever unshaded protobuf runtime the Spark driver provides. | ||
| _simple_desc_bytes = bytes.fromhex( | ||
| "0a360a0c73696d706c652e70726f746f12047465737422200a0653696d706c65" | ||
| "120b0a0369333218012001280512090a0173180220012809") | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def simple_desc(local_tmp_path): | ||
| # from_protobuf reads descFilePath via java.io.File on the driver. | ||
| desc_path = local_tmp_path + "/simple.desc" | ||
| with open(desc_path, "wb") as fp: | ||
| fp.write(_simple_desc_bytes) | ||
| return desc_path, _simple_desc_bytes | ||
|
|
||
|
|
||
| _smoke_rows = [(1, "a"), (-2, "bb"), (0, ""), (12345, "hello")] | ||
|
|
||
|
|
||
| def _make_smoke_df(spark): | ||
| encoded = [(_encode_simple_message(i, s),) for (i, s) in _smoke_rows] | ||
| return spark.createDataFrame(encoded, ["bin"]) | ||
|
|
||
|
|
||
| @allow_non_gpu("ProjectExec", "ProtobufDataToCatalyst") | ||
| def test_from_protobuf_smoke_path_api(simple_desc, from_protobuf_fn): | ||
| desc_path, _ = simple_desc | ||
|
|
||
| def run(spark): | ||
| return _make_smoke_df(spark).select( | ||
| from_protobuf_fn(f.col("bin"), "test.Simple", desc_path).alias("d")) | ||
|
|
||
| assert_gpu_fallback_collect(run, "ProtobufDataToCatalyst") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both tests compare CPU against GPU-with-fallback, and |
||
|
|
||
|
|
||
| @allow_non_gpu("ProjectExec", "ProtobufDataToCatalyst") | ||
| def test_from_protobuf_smoke_binary_descriptor_api(simple_desc, from_protobuf_fn): | ||
| if "binaryDescriptorSet" not in inspect.signature(from_protobuf_fn).parameters: | ||
| pytest.skip("binaryDescriptorSet kwarg is Spark 3.5+ only") | ||
| _, desc_bytes = simple_desc | ||
|
|
||
| def run(spark): | ||
| return _make_smoke_df(spark).select( | ||
| from_protobuf_fn(f.col("bin"), "test.Simple", | ||
| binaryDescriptorSet=bytearray(desc_bytes)).alias("d")) | ||
|
|
||
| assert_gpu_fallback_collect(run, "ProtobufDataToCatalyst") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing in this diff imports
protobuf; the tests deliberately use a static hex descriptor to avoid the dependency. Since the images have to be rebuilt after merge before it is even available, it seems to belong in the follow-up that needs it rather than here.