-
Notifications
You must be signed in to change notification settings - Fork 28k
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-42930][CORE][SQL] Change the access scope of ProtobufSerDe
related implementations to private[protobuf]
#40560
Conversation
ProtobufSerDe
related implementations to private[spark]
ProtobufSerDe
related implementations to private[spark]
@@ -25,7 +25,7 @@ import org.apache.spark.status.api.v1.AccumulableInfo | |||
import org.apache.spark.status.protobuf.Utils.{getOptional, getStringField, setStringField} | |||
import org.apache.spark.util.Utils.weakIntern | |||
|
|||
private[protobuf] object AccumulableInfoSerializer { | |||
private[spark] object AccumulableInfoSerializer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @gengliangwang @dongjoon-hyun FYI
Do you prefer private[spark]
or private[protobuf]
?
Should ProtobufSerDe
also be changed to private[spark]
? Although it has been identified as @DeveloperApi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Developer APIs are fine.
Do you prefer private[spark] or private[protobuf]?
Whichever narrower should be preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, Let me change to private[protobuf]
ProtobufSerDe
related implementations to private[spark]
ProtobufSerDe
related implementations to private[protobuf]
There was a problem hiding this 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 and @HyukjinKwon .
Merged to master/3.4.
…elated implementations to `private[protobuf]` ### What changes were proposed in this pull request? After [SPARK-41053](https://issues.apache.org/jira/browse/SPARK-41053), Spark supports serializing/ Live UI data to RocksDB using protobuf, but these are internal implementation details, so this pr change the access scope of `ProtobufSerDe` related implementations to `private[protobuf]`. ### Why are the changes needed? Weaker the access scope of Spark internal implementation details. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass GitHub Actions Closes #40560 from LuciferYang/SPARK-42930. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit b8f16bc) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Thanks @dongjoon-hyun @HyukjinKwon |
+1, @LuciferYang Thanks for the work! |
Thanks @gengliangwang ~ |
…elated implementations to `private[protobuf]` ### What changes were proposed in this pull request? After [SPARK-41053](https://issues.apache.org/jira/browse/SPARK-41053), Spark supports serializing/ Live UI data to RocksDB using protobuf, but these are internal implementation details, so this pr change the access scope of `ProtobufSerDe` related implementations to `private[protobuf]`. ### Why are the changes needed? Weaker the access scope of Spark internal implementation details. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass GitHub Actions Closes apache#40560 from LuciferYang/SPARK-42930. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit b8f16bc) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
What changes were proposed in this pull request?
After SPARK-41053, Spark supports serializing/ Live UI data to RocksDB using protobuf, but these are internal implementation details, so this pr change the access scope of
ProtobufSerDe
related implementations toprivate[protobuf]
.Why are the changes needed?
Weaker the access scope of Spark internal implementation details.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Pass GitHub Actions