Skip to content

Conversation

@LuciferYang
Copy link
Contributor

What changes were proposed in this pull request?

This pr includes the following changes:

  1. Refactor the dev/connect-gen-protos.sh script to support the generation of .py files from .proto files for both the connect and streaming modules simultaneously. Rename the script to dev/gen-protos.sh. Additionally, to maintain compatibility with previous development practices, this pull request (PR) introduces dev/connect-gen-protos.sh and dev/streaming-gen-protos.sh as wrappers around dev/gen-protos.sh.

After this PR, you can use:

dev/gen-protos.sh connect
dev/gen-protos.sh streaming

or

dev/connect-gen-protos.sh
dev/streaming-gen-protos.sh

to regenerate the corresponding .py files for the respective modules.

  1. Refactor the dev/connect-check-protos.py script to check the generated results for both the connect and streaming modules simultaneously, and rename it to dev/check-protos.py. Additionally, update the invocation of the check script in build_and_test.yml.

Why are the changes needed?

Provid tools for re-generate and checking StateMessage_pb2.py and StateMessage_pb2.pyi.

Does this PR introduce any user-facing change?

No

How was this patch tested?

  • Pass GitHub Actions

Was this patch authored or co-authored using generative AI tooling?

No

@LuciferYang
Copy link
Contributor Author

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @LuciferYang .

# source: StateMessage.proto
# Protobuf Python Version: 5.27.3
# source: org/apache/spark/sql/execution/streaming/StateMessage.proto
# Protobuf Python Version: 5.28.3
Copy link
Contributor

Choose a reason for hiding this comment

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

@jingz-db @bogao007 could you please take a look at the changes in generated files? since you touched these files recently

Copy link
Contributor

Choose a reason for hiding this comment

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

The changes LGTM, thanks!

@dongjoon-hyun
Copy link
Member

Thank you, @LuciferYang , @zhengruifeng , @bogao007 .

Merged to master for Apache Spark 4.0.0 on February 2024.

@LuciferYang
Copy link
Contributor Author

Thanks @dongjoon-hyun @zhengruifeng and @bogao007

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants