[SPARK-48639][CONNECT][PYTHON] Add Origin to Relation.RelationCommon #47024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HyukjinKwon
commented
Jun 19, 2024
| // (Required) Shared relation metadata. | ||
| string source_info = 1; | ||
| // (Optional) Shared relation metadata. | ||
| string source_info = 1 [deprecated=true]; |
Member
Author
There was a problem hiding this comment.
Seems source_info is unused yet in the codebase ... ?
Contributor
There was a problem hiding this comment.
why do we add this field at the first place?
Member
Author
There was a problem hiding this comment.
I think we wanted to leverage it somewhere codepoint but I think we ended up with not using it. @grundprinzip
Contributor
There was a problem hiding this comment.
I wanted to do what origin does, I had all prototypes etc ready, but never time. You can simply remove this field and mark the id reserved.
Suggested change
| string source_info = 1 [deprecated=true]; | |
| reserved 1; |
Member
Author
There was a problem hiding this comment.
This actually complains that it's a breaking change:
Run bufbuild/buf-breaking-action@v1
Error: Previously present field "1" with name "source_info" on message "RelationCommon" was deleted.
Error: buf found 1 breaking changes.
I can port this change to branch-3.5 if that's fine.
…ions.proto Co-authored-by: Martin Grund <[email protected]>
Member
Author
|
Merged to master and branch-3.5 partially |
HyukjinKwon
added a commit
that referenced
this pull request
Jun 24, 2024
This PR proposes to add `Origin` (from #46789) to `Relation.RelationCommon` To have the common protobuf message to keep the source code info. No. This is not used. CI should validate protobuf definition, and exiting tests should pass. No. Closes #47024 from HyukjinKwon/SPARK-48639. Lead-authored-by: Hyukjin Kwon <[email protected]> Co-authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]> (cherry picked from commit 09cb592) Signed-off-by: Hyukjin Kwon <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This PR proposes to add
Origin(from #46789) toRelation.RelationCommonWhy are the changes needed?
To have the common protobuf message to keep the source code info.
Does this PR introduce any user-facing change?
No. This is not used.
How was this patch tested?
CI should validate protobuf definition, and exiting tests should pass.
Was this patch authored or co-authored using generative AI tooling?
No.