You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/10/14 06:56:04 UTC
[GitHub] [spark] zhengruifeng opened a new pull request, #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
zhengruifeng opened a new pull request, #38253:
URL: https://github.com/apache/spark/pull/38253
### What changes were proposed in this pull request?
Check the generated python protos in GitHub Actions
### Why are the changes needed?
to make sure the generated python protos are latest
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
manually tests and CI
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng closed pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng closed pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
URL: https://github.com/apache/spark/pull/38253
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38253:
URL: https://github.com/apache/spark/pull/38253#issuecomment-1280345401
Let me merge it first to make sure connect protos are in sync
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] amaliujia commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r996250464
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -382,7 +386,9 @@ def plan(self, session: Optional["RemoteSparkSession"]) -> proto.Relation:
rel = proto.Relation()
rel.join.left.CopyFrom(self.left.plan(session))
rel.join.right.CopyFrom(self.right.plan(session))
- rel.join.on.CopyFrom(self.to_attr_or_expression(self.on, session))
+ rel.join.on.CopyFrom( # type: ignore[attr-defined]
Review Comment:
maybe different version of `mypy` so that you add to add this?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38253:
URL: https://github.com/apache/spark/pull/38253#issuecomment-1280324428
protos in existing master are out of sync, and are successfully detected by this PR:
```
Start checking the generated codes in pyspark-connect.
RUN: /__w/spark/spark/connector/connect/dev/generate_protos.sh /tmp/tmpme0gkjlx
Different files: ['commands_pb2.py', 'commands_pb2.pyi']
Generated files for pyspark-connect are out of sync! Please run ./connector/connect/dev/generate_protos.sh
Error: Process completed with exit code 255.
```
So I think it's ready
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r996571620
##########
dev/check-codegen-python.py:
##########
@@ -0,0 +1,81 @@
+#!/usr/bin/env python3
+
+#
+# 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.
+#
+
+# Utility for checking whether generated codes in PySpark are out of sync.
+# usage: ./dev/check-codegen-python.py
+
+import os
+import sys
+import filecmp
+import tempfile
+import subprocess
+
+# Location of your Spark git development area
+SPARK_HOME = os.environ.get("SPARK_HOME", os.getcwd())
+
+
+def fail(msg):
+ print(msg)
+ sys.exit(-1)
+
+
+def run_cmd(cmd):
+ print(f"RUN: {cmd}")
+ if isinstance(cmd, list):
+ return subprocess.check_output(cmd).decode("utf-8")
+ else:
+ return subprocess.check_output(cmd.split(" ")).decode("utf-8")
+
+
+def check_connect_protos():
+ print("Start checking the generated codes in pyspark-connect")
+ with tempfile.TemporaryDirectory() as tmp:
+ run_cmd(f"{SPARK_HOME}/connector/connect/dev/generate_protos.sh {tmp}")
+ result = filecmp.dircmp(f"{SPARK_HOME}/python/pyspark/sql/connect/proto/", tmp)
+ failed = False
+
+ if result.left_only != ["__init__.py"]:
+ files = [file for file in result.left_only if file != "__init__.py"]
+ print(f"Unexpected files: {files}")
+ failed = True
+
+ if len(result.right_only) > 0:
+ print(f"Missing files: {result.right_only}")
+ failed = True
+
+ if len(result.funny_files) > 0:
+ print(f"Incomparable files: {result.funny_files}")
+ failed = True
+
+ if len(result.diff_files) > 0:
+ print(f"Different files: {result.diff_files}")
+ failed = True
+
+ if not failed:
Review Comment:
let me update it
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38253:
URL: https://github.com/apache/spark/pull/38253#issuecomment-1280125891
installing `mypy-protobuf` doesn't change the versions of `mypy`,`mypy-extensions`,`pytest-mypy-plugins`, but it changes the behaviors:
```
(spark_test) ➜ spark git:(master) ✗ pip list | grep -i mypy
mypy 0.920
mypy-extensions 0.4.3
pytest-mypy-plugins 1.9.3
(spark_test) ➜ spark git:(master) ✗ ./dev/lint-python
starting python compilation test...
python compilation succeeded.
starting black test...
black checks passed.
starting flake8 test...
flake8 checks passed.
starting mypy annotations test...
annotations failed mypy checks:
python/pyspark/pandas/frame.py:6371: error: unused "type: ignore" comment
python/pyspark/pandas/frame.py:6428: error: unused "type: ignore" comment
python/pyspark/pandas/frame.py:6430: error: unused "type: ignore" comment
python/pyspark/pandas/resample.py:104: error: unused "type: ignore" comment
python/pyspark/pandas/resample.py:137: error: unused "type: ignore" comment
Found 5 errors in 2 files (checked 366 source files)
1
(spark_test) ➜ spark git:(master) ✗ pip install mypy-protobuf
Collecting mypy-protobuf
Using cached mypy_protobuf-3.3.0-py3-none-any.whl (16 kB)
Collecting types-protobuf>=3.19.12
Using cached types_protobuf-3.20.4-py3-none-any.whl (60 kB)
Requirement already satisfied: protobuf>=3.19.4 in /Users/ruifeng.zheng/.dev/miniconda3/envs/spark_test/lib/python3.10/site-packages (from mypy-protobuf) (4.21.6)
Installing collected packages: types-protobuf, mypy-protobuf
Successfully installed mypy-protobuf-3.3.0 types-protobuf-3.20.4
(spark_test) ➜ spark git:(master) ✗ ./dev/lint-python
starting python compilation test...
python compilation succeeded.
starting black test...
black checks passed.
starting flake8 test...
flake8 checks passed.
starting mypy annotations test...
annotations failed mypy checks:
python/pyspark/pandas/frame.py:6371: error: unused "type: ignore" comment
python/pyspark/pandas/frame.py:6428: error: unused "type: ignore" comment
python/pyspark/pandas/frame.py:6430: error: unused "type: ignore" comment
python/pyspark/pandas/resample.py:104: error: unused "type: ignore" comment
python/pyspark/pandas/resample.py:137: error: unused "type: ignore" comment
python/pyspark/sql/connect/plan.py:325: error: "AggregateFunction" has no attribute "function" [attr-defined]
python/pyspark/sql/connect/plan.py:327: error: "AggregateFunction" has no attribute "function" [attr-defined]
python/pyspark/sql/connect/plan.py:329: error: "AggregateFunction" has no attribute "function" [attr-defined]
python/pyspark/sql/connect/plan.py:338: error: "Aggregate" has no attribute "measures" [attr-defined]
python/pyspark/sql/connect/plan.py:342: error: "Type[Aggregate]" has no attribute "GroupingSet" [attr-defined]
python/pyspark/sql/connect/plan.py:344: error: "Aggregate" has no attribute "grouping_sets" [attr-defined]
python/pyspark/sql/connect/plan.py:385: error: "Join" has no attribute "on" [attr-defined]
Found 12 errors in 3 files (checked 366 source files)
1
(spark_test) ➜ spark git:(master) pip list | grep -i mypy
mypy 0.920
mypy-extensions 0.4.3
mypy-protobuf 3.3.0
pytest-mypy-plugins 1.9.3
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] amaliujia commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r996639372
##########
dev/check-codegen-python.py:
##########
@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+
+#
+# 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.
+#
+
+# Utility for checking whether generated codes in PySpark are out of sync.
+# usage: ./dev/check-codegen-python.py
+
+import os
+import sys
+import filecmp
+import tempfile
+import subprocess
+
+# Location of your Spark git development area
+SPARK_HOME = os.environ.get("SPARK_HOME", os.getcwd())
+
+
+def fail(msg):
+ print(msg)
+ sys.exit(-1)
+
+
+def run_cmd(cmd):
+ print(f"RUN: {cmd}")
+ if isinstance(cmd, list):
+ return subprocess.check_output(cmd).decode("utf-8")
+ else:
+ return subprocess.check_output(cmd.split(" ")).decode("utf-8")
+
+
+def check_connect_protos():
+ print("Start checking the generated codes in pyspark-connect.")
+ with tempfile.TemporaryDirectory() as tmp:
+ run_cmd(f"{SPARK_HOME}/connector/connect/dev/generate_protos.sh {tmp}")
+ result = filecmp.dircmp(
+ f"{SPARK_HOME}/python/pyspark/sql/connect/proto/",
+ tmp,
+ ignore=["__init__.py", "__pycache__"],
+ )
+ success = True
+
+ if len(result.left_only) > 0:
+ print(f"Unexpected files: {result.left_only}")
+ success = False
+
+ if len(result.right_only) > 0:
+ print(f"Missing files: {result.right_only}")
+ success = False
+
+ if len(result.funny_files) > 0:
+ print(f"Incomparable files: {result.funny_files}")
+ success = False
+
+ if len(result.diff_files) > 0:
+ print(f"Different files: {result.diff_files}")
+ success = False
+
+ if success:
+ print("Finish checking the generated codes in pyspark-connect: SUCCESS")
+ else:
+ fail(
+ "Generated files for pyspark-connect are out of sync! "
+ "Please run ./connector/connect/dev/generate_protos.sh"
+ )
+
+
+check_connect_protos()
+
+# TODO: also check generated code in pyspark-ml and pyspark-mllib.
Review Comment:
Nit:
maybe create a JIRA for this and link the JIRA number?
##########
dev/check-codegen-python.py:
##########
@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+
+#
+# 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.
+#
+
+# Utility for checking whether generated codes in PySpark are out of sync.
+# usage: ./dev/check-codegen-python.py
+
+import os
+import sys
+import filecmp
+import tempfile
+import subprocess
+
+# Location of your Spark git development area
+SPARK_HOME = os.environ.get("SPARK_HOME", os.getcwd())
+
+
+def fail(msg):
+ print(msg)
+ sys.exit(-1)
+
+
+def run_cmd(cmd):
+ print(f"RUN: {cmd}")
+ if isinstance(cmd, list):
+ return subprocess.check_output(cmd).decode("utf-8")
+ else:
+ return subprocess.check_output(cmd.split(" ")).decode("utf-8")
+
+
+def check_connect_protos():
+ print("Start checking the generated codes in pyspark-connect.")
+ with tempfile.TemporaryDirectory() as tmp:
+ run_cmd(f"{SPARK_HOME}/connector/connect/dev/generate_protos.sh {tmp}")
+ result = filecmp.dircmp(
+ f"{SPARK_HOME}/python/pyspark/sql/connect/proto/",
+ tmp,
+ ignore=["__init__.py", "__pycache__"],
+ )
+ success = True
+
+ if len(result.left_only) > 0:
+ print(f"Unexpected files: {result.left_only}")
+ success = False
+
+ if len(result.right_only) > 0:
+ print(f"Missing files: {result.right_only}")
+ success = False
+
+ if len(result.funny_files) > 0:
+ print(f"Incomparable files: {result.funny_files}")
+ success = False
+
+ if len(result.diff_files) > 0:
+ print(f"Different files: {result.diff_files}")
+ success = False
+
+ if success:
+ print("Finish checking the generated codes in pyspark-connect: SUCCESS")
+ else:
+ fail(
+ "Generated files for pyspark-connect are out of sync! "
+ "Please run ./connector/connect/dev/generate_protos.sh"
Review Comment:
Nit:
I guess people will hit `mypy-protobuf` module not installed issue thus the script will fail. It might not be obvious for people to discover that this requires a `pip install`.
Do we need to remind people check the `dev/requirement.txt` or we can trust that people will figure it out without a need for a pointer?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38253:
URL: https://github.com/apache/spark/pull/38253#issuecomment-1278976198
I manually modify `expressions_pb2.py` and then this check failed https://github.com/zhengruifeng/spark/actions/runs/3249624241/jobs/5332272955
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] amaliujia commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r996250933
##########
dev/check-codegen-python.py:
##########
@@ -0,0 +1,81 @@
+#!/usr/bin/env python3
+
+#
+# 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.
+#
+
+# Utility for checking whether generated codes in PySpark are out of sync.
+# usage: ./dev/check-codegen-python.py
+
+import os
+import sys
+import filecmp
+import tempfile
+import subprocess
+
+# Location of your Spark git development area
+SPARK_HOME = os.environ.get("SPARK_HOME", os.getcwd())
+
+
+def fail(msg):
+ print(msg)
+ sys.exit(-1)
+
+
+def run_cmd(cmd):
+ print(f"RUN: {cmd}")
+ if isinstance(cmd, list):
+ return subprocess.check_output(cmd).decode("utf-8")
+ else:
+ return subprocess.check_output(cmd.split(" ")).decode("utf-8")
+
+
+def check_connect_protos():
+ print("Start checking the generated codes in pyspark-connect")
+ with tempfile.TemporaryDirectory() as tmp:
+ run_cmd(f"{SPARK_HOME}/connector/connect/dev/generate_protos.sh {tmp}")
+ result = filecmp.dircmp(f"{SPARK_HOME}/python/pyspark/sql/connect/proto/", tmp)
+ failed = False
+
+ if result.left_only != ["__init__.py"]:
+ files = [file for file in result.left_only if file != "__init__.py"]
+ print(f"Unexpected files: {files}")
+ failed = True
+
+ if len(result.right_only) > 0:
+ print(f"Missing files: {result.right_only}")
+ failed = True
+
+ if len(result.funny_files) > 0:
+ print(f"Incomparable files: {result.funny_files}")
+ failed = True
+
+ if len(result.diff_files) > 0:
+ print(f"Different files: {result.diff_files}")
+ failed = True
+
+ if not failed:
Review Comment:
nit:
I usually suggest to do
```
if condition is true
then else
```
so it is
```
if failed:
else:
```
but this is minor.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38253:
URL: https://github.com/apache/spark/pull/38253#issuecomment-1280325547
cc @HyukjinKwon @amaliujia @grundprinzip please take another look
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on PR #38253:
URL: https://github.com/apache/spark/pull/38253#issuecomment-1280346449
merged into master
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r995420859
##########
connector/connect/dev/generate_protos.sh:
##########
@@ -55,10 +70,13 @@ buf generate --debug -vvv
for f in `find gen/proto/python -name "*.py*"`; do
# First fix the imports.
if [[ $f == *_pb2.py || $f == *_pb2_grpc.py ]]; then
- sed -i '' -e 's/from spark.connect import/from pyspark.sql.connect.proto import/g' $f
Review Comment:
`sed -i ''` doesn't work in Linux, so make this change
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r996895976
##########
.github/workflows/build_and_test.yml:
##########
@@ -601,6 +607,8 @@ jobs:
run: ./dev/lint-java
- name: Python linter
run: PYTHON_EXECUTABLE=python3.9 ./dev/lint-python
+ - name: Python code generation check
+ run: PATH=$PATH:$HOME/buf/bin PYTHON_EXECUTABLE=python3.9 ./dev/check-codegen-python.py
Review Comment:
got it, let me fix it
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] grundprinzip commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD][WIP] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r996130535
##########
dev/check-codegen-python.py:
##########
@@ -0,0 +1,81 @@
+#!/usr/bin/env python3
+
+#
+# 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.
+#
+
+# Utility for checking whether generated codes in PySpark are out of sync.
+# usage: ./dev/check-codegen-python.py
+
+import os
+import sys
+import filecmp
+import tempfile
+import subprocess
+
+# Location of your Spark git development area
+SPARK_HOME = os.environ.get("SPARK_HOME", os.getcwd())
+
+
+def fail(msg):
+ print(msg)
+ sys.exit(-1)
+
+
+def run_cmd(cmd):
+ print(f"RUN: {cmd}")
+ if isinstance(cmd, list):
+ return subprocess.check_output(cmd).decode("utf-8")
+ else:
+ return subprocess.check_output(cmd.split(" ")).decode("utf-8")
+
+
+def check_connect_protos():
+ print("Start checking the generated codes in pyspark-connect")
+ with tempfile.TemporaryDirectory() as tmp:
+ run_cmd(f"{SPARK_HOME}/connector/connect/dev/generate_protos.sh {tmp}")
+ result = filecmp.dircmp(f"{SPARK_HOME}/python/pyspark/sql/connect/proto/", tmp)
+ failed = False
+
+ if result.left_only != ["__init__.py"]:
+ files = [file for file in result.left_only if file != "__init__.py"]
+ print(f"Unexpected files: {files}")
+ failed = True
+
+ if len(result.right_only) > 0:
+ print(f"Missing files: {result.right_only}")
+ failed = True
+
+ if len(result.funny_files) > 0:
+ print(f"Incomparable files: {result.funny_files}")
+ failed = True
+
+ if len(result.diff_files) > 0:
+ print(f"Different files: {result.diff_files}")
+ failed = True
+
+ if not failed:
+ print("Finish checking the generated codes in pyspark-connect: SUCCESS")
+ else:
+ fail(
+ "Generated codes for pyspark-connect are out of sync! "
+ "Please run ./connector/connect/dev/generate_protos.sh"
+ )
+
+
+check_connect_protos()
+
+# TODO: also check generated codes in pyspark-ml and pyspark-mllib
Review Comment:
```suggestion
# TODO: also check generated code in pyspark-ml and pyspark-mllib.
```
##########
python/pyspark/sql/connect/plan.py:
##########
@@ -322,11 +322,15 @@ def _convert_measure(
) -> proto.Aggregate.AggregateFunction:
exp, fun = m
measure = proto.Aggregate.AggregateFunction()
- measure.function.name = fun
+ measure.function.name = fun # type: ignore[attr-defined]
Review Comment:
Why do these show up as required now, even though we had mypy checks activated before>
##########
dev/check-codegen-python.py:
##########
@@ -0,0 +1,81 @@
+#!/usr/bin/env python3
+
+#
+# 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.
+#
+
+# Utility for checking whether generated codes in PySpark are out of sync.
+# usage: ./dev/check-codegen-python.py
+
+import os
+import sys
+import filecmp
+import tempfile
+import subprocess
+
+# Location of your Spark git development area
+SPARK_HOME = os.environ.get("SPARK_HOME", os.getcwd())
+
+
+def fail(msg):
+ print(msg)
+ sys.exit(-1)
+
+
+def run_cmd(cmd):
+ print(f"RUN: {cmd}")
+ if isinstance(cmd, list):
+ return subprocess.check_output(cmd).decode("utf-8")
+ else:
+ return subprocess.check_output(cmd.split(" ")).decode("utf-8")
+
+
+def check_connect_protos():
+ print("Start checking the generated codes in pyspark-connect")
Review Comment:
```suggestion
print("Start checking the generated codes in pyspark-connect.")
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #38253: [SPARK-40796][CONNECT][BUILD] Check the generated python protos in GitHub Actions
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #38253:
URL: https://github.com/apache/spark/pull/38253#discussion_r996888620
##########
.github/workflows/build_and_test.yml:
##########
@@ -601,6 +607,8 @@ jobs:
run: ./dev/lint-java
- name: Python linter
run: PYTHON_EXECUTABLE=python3.9 ./dev/lint-python
+ - name: Python code generation check
+ run: PATH=$PATH:$HOME/buf/bin PYTHON_EXECUTABLE=python3.9 ./dev/check-codegen-python.py
Review Comment:
Let's skip this test if the script doesn't exist. This workflow runs in branch-3.2 and 3.3 too (see https://github.com/apache/spark/actions/runs/3263093569/jobs/5361307965)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org