Skip to content
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

[SPARK-51142][ML][CONNECT] ML protobufs clean up #49862

Closed
wants to merge 4 commits into from

Conversation

zhengruifeng
Copy link
Contributor

What changes were proposed in this pull request?

ML protobufs clean up

Why are the changes needed?

to follow the guide https://github.com/apache/spark/blob/ece14704cc083f17689d2e0b9ab8e31cf71a7a2d/sql/connect/docs/adding-proto-messages.md

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

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

no

TRANSFORMER = 2;
EVALUATOR = 3;
MODEL = 4;
OPERATOR_TYPE_UNSPECIFIED = 0;
Copy link
Contributor Author

@zhengruifeng zhengruifeng Feb 10, 2025

Choose a reason for hiding this comment

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

rename to follow the protobuf style guide

Prefix every value with the enum name (converted to UPPER_SNAKE_CASE)

https://protobuf.dev/programming-guides/style/#enums

@zhengruifeng
Copy link
Contributor Author

cc @grundprinzip and @wbo4958

@zhengruifeng
Copy link
Contributor Author

The failed protobuf breaking change test is expected, we will merge this also in 4.0

Copy link
Contributor

@grundprinzip grundprinzip left a comment

Choose a reason for hiding this comment

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

Thanks for doing this before the release!!! Much appreciated!

@zhengruifeng
Copy link
Contributor Author

I am going to merge this into 4.0, otherwise the protobuf compatibility between 4.0 and 4.1 will be broken

zhengruifeng added a commit that referenced this pull request Feb 11, 2025
### What changes were proposed in this pull request?
ML protobufs clean up

### Why are the changes needed?
to follow the guide https://github.com/apache/spark/blob/ece14704cc083f17689d2e0b9ab8e31cf71a7a2d/sql/connect/docs/adding-proto-messages.md

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
existing tests

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

Closes #49862 from zhengruifeng/ml_connect_protos.

Authored-by: Ruifeng Zheng <ruifengz@apache.org>
Signed-off-by: Ruifeng Zheng <ruifengz@apache.org>
(cherry picked from commit 6a71f76)
Signed-off-by: Ruifeng Zheng <ruifengz@apache.org>
@zhengruifeng
Copy link
Contributor Author

merged to master/4.0

@zhengruifeng zhengruifeng deleted the ml_connect_protos branch February 11, 2025 01:54
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.

3 participants