Skip to content

[SPARK-40796][CONNECT][FOLLOW-UP] Improve README for proto generated files in Connect Python client #38335

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.

Sign up for GitHub

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

Closed
amaliujia wants to merge 4 commits into from
Closed

[SPARK-40796][CONNECT][FOLLOW-UP] Improve README for proto generated files in Connect Python client #38335

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a915e6d
[SPARK-40796][CONNECT][FOLLOW-UP] Improve README for proto generated …
amaliujia Oct 21, 2022
aadb52a
update
amaliujia Oct 21, 2022
d78f4a9
update
amaliujia Oct 22, 2022
43ded3d
update
amaliujia Oct 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion dev/check-codegen-python.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ def check_connect_protos():
else:
fail(
"Generated files for pyspark-connect are out of sync! "
"Please run ./connector/connect/dev/generate_protos.sh"
"If you have touched files under connector/connect/src/main/protobuf, "
"please run ./connector/connect/dev/generate_protos.sh. "
"If you haven't touched any file above, please rebase your PR against main branch."
)


Expand Down
5 changes: 5 additions & 0 deletions python/pyspark/sql/connect/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,8 @@ To use the release version of Spark Connect:
./python/run-tests --testnames 'pyspark.sql.tests.connect.test_connect_basic'
```

## Generate proto generated files for the Python client
1. Install `buf`: https://docs.buf.build/installation
Copy link
Contributor

@zhengruifeng zhengruifeng Oct 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to mention the related python packages:

grpcio==1.48.1
protobuf==4.21.6
mypy-protobuf==3.3.0

Copy link
Contributor

@zhengruifeng zhengruifeng Oct 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buf's version in CI is 1.8.0, the latest one is 1.9.0. I think we also need to mention the version

Copy link
Contributor Author

@amaliujia amaliujia Oct 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. I updated this doc with versions.

2. Run `pip install grpcio==1.48.1 protobuf==4.21.6 mypy-protobuf==3.3.0`
3. Run `./dev/generate_protos.sh`
Copy link
Contributor

@zhengruifeng zhengruifeng Oct 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, should be ./connector/connect/dev/generate_protos.sh

Copy link
Contributor

@zhengruifeng zhengruifeng Oct 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amaliujia

Copy link
Contributor

@zhengruifeng zhengruifeng Oct 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fix it in #38339

Copy link
Contributor Author

@amaliujia amaliujia Oct 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops I copied this from my local command which is not under the spark repo root directory.

@zhengruifeng maybe you can BTW update it in the PR that you pin dependency versions?