[SPARK-40914][CONNECT] Mark org.apache.spark.sql.connect to be private
#38392
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
amaliujia
changed the title
HyukjinKwon
reviewed
Oct 25, 2022
| package object dsl { | ||
|
|
||
| object expressions { // scalastyle:ignore | ||
| private[connect] object expressions { // scalastyle:ignore |
There was a problem hiding this comment.
If we document this as an internal package, I think it's fine to remove private[connect], see also SPARK-16964 and SPARK-16813
There was a problem hiding this comment.
Makes sense at least for this dsl package. Removed private[connect] in this file.
HyukjinKwon
approved these changes
Oct 26, 2022
|
Thinking about this more, I think only the client is the user-facing API of Spark Connect. We can mark the entire |
|
This also makes sense. Proto is API but server is not. Can you share an example of how to mark the entire package as |
|
For catalyst, what we did is to not generate API doc for it: https://github.com/apache/spark/blob/master/project/SparkBuild.scala#L1179 No public doc means everything in this package is private. |
itholic
approved these changes
Oct 26, 2022
|
Can one of the admins verify this patch? |
There was a problem hiding this comment.
+1 for @cloud-fan 's opinion about preventing document generations.
amaliujia
changed the title
amaliujia
changed the title
|
Thanks everyone. I choose to apply the suggestion that mark the entire server module as private. The server implementation shouldn't be public API anyway. Only proto and DataFrames in clients are public API. |
amaliujia
commented
Oct 26, 2022
| * A collection of implicit conversions that create a DSL for constructing connect protos. | ||
| * | ||
| * All classes in connect/dsl are considered an internal API to Spark Connect and are subject to | ||
| * change between minor releases. |
There was a problem hiding this comment.
I still keep this to match Catalyst's Dsl.
dongjoon-hyun
changed the title
org.apache.spark.sql.connect to be private
HyukjinKwon
approved these changes
Oct 27, 2022
|
Merged to master. The failed test is completely unrelated to this change. |
SandishKumarHN
pushed a commit
to SandishKumarHN/spark
that referenced
this pull request
Dec 12, 2022
### What changes were proposed in this pull request? This PR proposes to mark non-public API as package private. E.g. private[connect]. ### Why are the changes needed? This is to control our API surface and don't expose non-public API. ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? UT Closes apache#38392 from amaliujia/SPARK-40914. Authored-by: Rui Wang <rui.wang@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
dongjoon-hyun
pushed a commit
that referenced
this pull request
Nov 17, 2023
### What changes were proposed in this pull request? This PR restores the DSv2 documentation. #38392 mistakenly added `org/apache/spark/sql/connect` as a private that includes `org/apache/spark/sql/connector`. ### Why are the changes needed? For end users to read DSv2 documentation. ### Does this PR introduce _any_ user-facing change? Yes, it restores the DSv2 API documentation that used to be there https://spark.apache.org/docs/3.3.0/api/scala/org/apache/spark/sql/connector/catalog/index.html ### How was this patch tested? Manually tested via: ``` SKIP_PYTHONDOC=1 SKIP_RDOC=1 SKIP_SQLDOC=1 bundle exec jekyll build ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #43855 from HyukjinKwon/connector-docs. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
HyukjinKwon
added a commit
to HyukjinKwon/spark
that referenced
this pull request
Nov 17, 2023
This PR restores the DSv2 documentation. apache#38392 mistakenly added `org/apache/spark/sql/connect` as a private that includes `org/apache/spark/sql/connector`. For end users to read DSv2 documentation. Yes, it restores the DSv2 API documentation that used to be there https://spark.apache.org/docs/3.3.0/api/scala/org/apache/spark/sql/connector/catalog/index.html Manually tested via: ``` SKIP_PYTHONDOC=1 SKIP_RDOC=1 SKIP_SQLDOC=1 bundle exec jekyll build ``` No. Closes apache#43855 from HyukjinKwon/connector-docs. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit a7147c8) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
HyukjinKwon
added a commit
to HyukjinKwon/spark
that referenced
this pull request
Nov 17, 2023
This PR restores the DSv2 documentation. apache#38392 mistakenly added `org/apache/spark/sql/connect` as a private that includes `org/apache/spark/sql/connector`. For end users to read DSv2 documentation. Yes, it restores the DSv2 API documentation that used to be there https://spark.apache.org/docs/3.3.0/api/scala/org/apache/spark/sql/connector/catalog/index.html Manually tested via: ``` SKIP_PYTHONDOC=1 SKIP_RDOC=1 SKIP_SQLDOC=1 bundle exec jekyll build ``` No. Closes apache#43855 from HyukjinKwon/connector-docs. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit a7147c8) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit bcfebb5) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
This was referenced Nov 17, 2023
dongjoon-hyun
pushed a commit
that referenced
this pull request
Nov 17, 2023
This PR cherry-picks #43855 to branch-3.5 --- ### What changes were proposed in this pull request? This PR restores the DSv2 documentation. #38392 mistakenly added `org/apache/spark/sql/connect` as a private that includes `org/apache/spark/sql/connector`. ### Why are the changes needed? For end users to read DSv2 documentation. ### Does this PR introduce _any_ user-facing change? Yes, it restores the DSv2 API documentation that used to be there https://spark.apache.org/docs/3.3.0/api/scala/org/apache/spark/sql/connector/catalog/index.html ### How was this patch tested? Manually tested via: ``` SKIP_PYTHONDOC=1 SKIP_RDOC=1 SKIP_SQLDOC=1 bundle exec jekyll build ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #43865 from HyukjinKwon/SPARK-45963-3.5. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
dongjoon-hyun
pushed a commit
that referenced
this pull request
Nov 18, 2023
This PR cherry-picks #43855 to branch-3.4 --- ### What changes were proposed in this pull request? This PR restores the DSv2 documentation. #38392 mistakenly added `org/apache/spark/sql/connect` as a private that includes `org/apache/spark/sql/connector`. ### Why are the changes needed? For end users to read DSv2 documentation. ### Does this PR introduce _any_ user-facing change? Yes, it restores the DSv2 API documentation that used to be there https://spark.apache.org/docs/3.3.0/api/scala/org/apache/spark/sql/connector/catalog/index.html ### How was this patch tested? Manually tested via: ``` SKIP_PYTHONDOC=1 SKIP_RDOC=1 SKIP_SQLDOC=1 bundle exec jekyll build ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #43866 from HyukjinKwon/SPARK-45963-3.4. Lead-authored-by: Hyukjin Kwon <gurwls223@apache.org> Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
szehon-ho
pushed a commit
to szehon-ho/spark
that referenced
this pull request
Feb 7, 2024
This PR cherry-picks apache#43855 to branch-3.4 --- ### What changes were proposed in this pull request? This PR restores the DSv2 documentation. apache#38392 mistakenly added `org/apache/spark/sql/connect` as a private that includes `org/apache/spark/sql/connector`. ### Why are the changes needed? For end users to read DSv2 documentation. ### Does this PR introduce _any_ user-facing change? Yes, it restores the DSv2 API documentation that used to be there https://spark.apache.org/docs/3.3.0/api/scala/org/apache/spark/sql/connector/catalog/index.html ### How was this patch tested? Manually tested via: ``` SKIP_PYTHONDOC=1 SKIP_RDOC=1 SKIP_SQLDOC=1 bundle exec jekyll build ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#43866 from HyukjinKwon/SPARK-45963-3.4. Lead-authored-by: Hyukjin Kwon <gurwls223@apache.org> Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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 mark non-public API as package private. E.g. private[connect].
Why are the changes needed?
This is to control our API surface and don't expose non-public API.
Does this PR introduce any user-facing change?
NO
How was this patch tested?
UT