Skip to content

Conversation

@gengliangwang
Copy link
Member

What changes were proposed in this pull request?

Compare the 3.2.0 API doc with the latest release version 3.1.2. Fix the following issues:

  • Add missing Since annotation for new APIs
  • Remove the leaking class/object in API doc

Why are the changes needed?

Improve API docs

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing UT

@gengliangwang gengliangwang changed the title [SPARK-36457][Doc] Review and fix issues in API docs [SPARK-36457][Docs] Review and fix issues in API docs Aug 24, 2021
* Arithmetic exception thrown from Spark with an error class.
*/
class SparkArithmeticException(errorClass: String, messageParameters: Array[String])
private[spark] class SparkArithmeticException(errorClass: String, messageParameters: Array[String])
Copy link
Member Author

Choose a reason for hiding this comment

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

* Object for grouping error messages from (most) exceptions thrown during query execution.
*/
object SparkCoreErrors {
private[spark] object SparkCoreErrors {
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

shall we mark org.apache.spark.errors as a private package as well?

* change the checksum calculator at runtime.
*/
class MutableCheckedOutputStream(out: OutputStream) extends OutputStream {
private[spark] class MutableCheckedOutputStream(out: OutputStream) extends OutputStream {
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

import org.apache.spark.internal.config._

case class IvyProperties(
private[spark] case class IvyProperties(
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

*
* @since 3.2.0
*/
@Evolving
Copy link
Member Author

Choose a reason for hiding this comment

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

*
* @since 3.2.0
*/
@Evolving
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Also please take a look at the following DSV2 changes, thanks!

* grouped into [[QueryCompilationErrors]].
*/
object QueryExecutionErrors {
private[spark] object QueryExecutionErrors {
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

shall we mark the entire org.apache.spark.sql.errors as private?

See https://github.com/apache/spark/blob/master/project/SparkBuild.scala#L944-L973

Copy link
Member Author

Choose a reason for hiding this comment

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

SGTM. @karenfeng will there be any new public API under org.apache.spark.sql.errors?

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @allisonwang-db

I think that the only useful API to expose related to this may be SparkThrowable, which contains errorClass and sqlState. But that is under org.apache.spark.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, let me mark org.apache.spark.sql.errors as private

* Currently it includes all ParseException.
*/
object QueryParsingErrors {
private[spark] object QueryParsingErrors {
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@SparkQA
Copy link

SparkQA commented Aug 24, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47231/

@SparkQA
Copy link

SparkQA commented Aug 24, 2021

Test build #142731 has finished for PR 33824 at commit df29e32.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon HyukjinKwon changed the title [SPARK-36457][Docs] Review and fix issues in API docs [SPARK-36457][DOCS] Review and fix issues in API docs Aug 25, 2021
@github-actions github-actions bot added the BUILD label Aug 25, 2021
@SparkQA
Copy link

SparkQA commented Aug 25, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47251/

@SparkQA
Copy link

SparkQA commented Aug 25, 2021

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47251/

@SparkQA
Copy link

SparkQA commented Aug 25, 2021

Test build #142752 has finished for PR 33824 at commit 3769688.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gengliangwang gengliangwang changed the title [SPARK-36457][DOCS] Review and fix issues in API docs [SPARK-36457][DOCS] Review and fix issues in Scala/Java API docs Aug 26, 2021
@gengliangwang
Copy link
Member Author

Merging to master/3.2

gengliangwang added a commit to gengliangwang/spark that referenced this pull request Aug 26, 2021
Compare the 3.2.0 API doc with the latest release version 3.1.2. Fix the following issues:

- Add missing `Since` annotation for new APIs
- Remove the leaking class/object in API doc

Improve API docs

No

Existing UT

Closes apache#33824 from gengliangwang/auditDoc.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
gengliangwang added a commit to gengliangwang/spark that referenced this pull request Aug 27, 2021
* the functions make_dt_interval and make_ym_interval should make it clear that some of the fields are optional
* remove the `|` symbol from the doc of `bit_get` https://dist.apache.org/repos/dist/dev/spark/v3.2.0-rc1-docs/_site/api/sql/index.html#bit_get
* Address one missing comment in apache#33824 (comment)

Improve the documentation.

No

Build doc and preview:
![image](https://user-images.githubusercontent.com/1097932/130996918-8c1fff88-ef5a-434b-8445-df7140bad3ba.png)
![image](https://user-images.githubusercontent.com/1097932/130996954-0ced28e7-fb90-4fcc-857e-6ccc31dc3c09.png)

![image](https://user-images.githubusercontent.com/1097932/130955106-5ae32dfc-6e89-4e28-bb8a-6c1b5213051c.png)

![image](https://user-images.githubusercontent.com/1097932/130922351-2f0f262d-5624-4d08-ba83-dfa3ed0b646b.png)

Closes apache#33847 from gengliangwang/auditSQLDoc.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Gengliang Wang <gengliang@apache.org>
dongjoon-hyun pushed a commit that referenced this pull request Aug 27, 2021
### What changes were proposed in this pull request?

* the functions make_dt_interval and make_ym_interval should make it clear that some of the fields are optional
* remove the `|` symbol from the doc of `bit_get` https://dist.apache.org/repos/dist/dev/spark/v3.2.0-rc1-docs/_site/api/sql/index.html#bit_get
* Address one missing comment in #33824 (comment)

### Why are the changes needed?

Improve the documentation.

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

No

### How was this patch tested?

Build doc and preview:
![image](https://user-images.githubusercontent.com/1097932/130996918-8c1fff88-ef5a-434b-8445-df7140bad3ba.png)
![image](https://user-images.githubusercontent.com/1097932/130996954-0ced28e7-fb90-4fcc-857e-6ccc31dc3c09.png)

![image](https://user-images.githubusercontent.com/1097932/130955106-5ae32dfc-6e89-4e28-bb8a-6c1b5213051c.png)

![image](https://user-images.githubusercontent.com/1097932/130922351-2f0f262d-5624-4d08-ba83-dfa3ed0b646b.png)

Closes #33857 from gengliangwang/SPARK-36597-3.2.

Authored-by: Gengliang Wang <gengliang@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
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.

8 participants