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.

Sign up for GitHub

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

Jump to bottom

[SPARK-23119][SS] Minor fixes to V2 streaming APIs #20286

Closed
wants to merge 2 commits into from
Closed

[SPARK-23119][SS] Minor fixes to V2 streaming APIs #20286

wants to merge 2 commits into from

Conversation

tdas
Copy link
Contributor

@tdas tdas commented Jan 17, 2018

What changes were proposed in this pull request?

  • Added @InterfaceStability.Evolving annotations
  • Improved docs.

How was this patch tested?

Existing tests.

@tdas
Copy link
Contributor Author

tdas commented Jan 17, 2018

@jose-torres @zsxwing Can you take a look.

@jose-torres
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Jan 17, 2018

Test build #86224 has finished for PR 20286 at commit f6dfa58.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@tdas
Copy link
Contributor Author

tdas commented Jan 17, 2018

jenkins retest this please.

zsxwing
zsxwing reviewed Jan 17, 2018
View reviewed changes
Copy link
Member

@zsxwing zsxwing left a comment

Choose a reason for hiding this comment

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

LGTM except some nits

sql/core/src/main/java/org/apache/spark/sql/sources/v2/streaming/reader/MicroBatchReader.java Outdated
@@ -25,7 +26,11 @@
/**
* A mix-in interface for {@link DataSourceV2Reader}. Data source readers can implement this
* interface to indicate they allow micro-batch streaming reads.
*
* Note: This class currently extends {@link BaseStreamingSource} to maintain compatibility with
* DataSource V1 APIs. This will be extension will be removed once we get rid of V1 completely.
Copy link
Member

Choose a reason for hiding this comment

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

ditto

sql/core/src/main/java/org/apache/spark/sql/sources/v2/streaming/reader/ContinuousReader.java Outdated
@@ -27,11 +28,15 @@
* interface to allow reading in a continuous processing mode stream.
*
* Implementations must ensure each read task output is a {@link ContinuousDataReader}.
*
* Note: This class currently extends {@link BaseStreamingSource} to maintain compatibility with
* DataSource V1 APIs. This will be extension will be removed once we get rid of V1 completely.
Copy link
Member

Choose a reason for hiding this comment

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

nit: This will be extension

sql/core/src/main/java/org/apache/spark/sql/sources/v2/streaming/reader/Offset.java Outdated
* to reconstruct a position in the stream up to which data has been seen/processed.
*
* Note: This class currently extends {@link org.apache.spark.sql.execution.streaming.Offset} to
* maintain compatibility with DataSource V1 APIs. This will be extension will be removed once we
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@SparkQA
Copy link

SparkQA commented Jan 17, 2018

Test build #86249 has finished for PR 20286 at commit f6dfa58.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

smurakozi
smurakozi reviewed Jan 17, 2018
View reviewed changes
Copy link
Contributor

@smurakozi smurakozi left a comment

Choose a reason for hiding this comment

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

I think this change is OK, except the nits zsxwing already noted.

@SparkQA
Copy link

SparkQA commented Jan 18, 2018

Test build #86292 has finished for PR 20286 at commit 3b3029b.

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

@zsxwing
Copy link
Member

zsxwing commented Jan 18, 2018

LGTM. Merging to master and 2.3.

asfgit pushed a commit that referenced this pull request Jan 18, 2018
@tdas @zsxwing
[SPARK-23119][SS] Minor fixes to V2 streaming APIs
9783aea 
## What changes were proposed in this pull request?

- Added `InterfaceStability.Evolving` annotations
- Improved docs.

## How was this patch tested?
Existing tests.

Author: Tathagata Das <[email protected]>

Closes #20286 from tdas/SPARK-23119.

(cherry picked from commit bac0d66)
Signed-off-by: Shixiong Zhu <[email protected]>
@asfgit asfgit closed this in bac0d66 Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smurakozi smurakozi smurakozi left review comments

@zsxwing zsxwing zsxwing left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tdas @jose-torres @SparkQA @zsxwing @smurakozi