Skip to content

Conversation

@cloud-fan
Copy link
Contributor

@cloud-fan cloud-fan commented Sep 11, 2018

What changes were proposed in this pull request?

As discussed in the dev list, we don't want to include #22009 in Spark 2.4, as it needs data source v2 users to change the implementation intensitively, while they need to change again in next release.

How was this patch tested?

existing tests

@cloud-fan
Copy link
Contributor Author

cloud-fan commented Sep 11, 2018

cc @rxin @rdblue @jose-torres

@SparkQA
Copy link

SparkQA commented Sep 11, 2018

Test build #95912 has finished for PR 22388 at commit e31ecfa.

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

@dongjoon-hyun
Copy link
Member

This seems to be the complete revert of SPARK-24882 (#22009 and the followup #22245). Am I right?

@cloud-fan
Copy link
Contributor Author

yes

@SparkQA
Copy link

SparkQA commented Sep 12, 2018

Test build #95967 has finished for PR 22388 at commit 234b67a.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 12, 2018

Test build #95968 has finished for PR 22388 at commit 031ad29.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member

kiszk commented Sep 12, 2018

Incorrect import still may exist there.

[error] /home/jenkins/workspace/SparkPullRequestBuilder/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/continuous/ContinuousTextSocketSource.scala:40: object SupportsDeprecatedScanRow is not a member of package org.apache.spark.sql.sources.v2.reader
[error] import org.apache.spark.sql.sources.v2.reader.{InputPartition, InputPartitionReader, SupportsDeprecatedScanRow}
[error]        ^

@jose-torres
Copy link
Contributor

MicroBatchExecution.scala and ContinuousExecution.scala look right after the revert, although it would be helpful to understand what the diff is between this and a straight git revert.

@cloud-fan
Copy link
Contributor Author

cloud-fan commented Sep 12, 2018

I can't recall the exact conflicts. There are only 2 commits(3106324 and 9f5c5b4) touched these 2 files after my PR, I carefully checked and these changs are still there.

@SparkQA
Copy link

SparkQA commented Sep 12, 2018

Test build #95986 has finished for PR 22388 at commit be13162.

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

Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks! Merged to 2.4.

asfgit pushed a commit that referenced this pull request Sep 12, 2018
## What changes were proposed in this pull request?

As discussed in the dev list, we don't want to include #22009 in Spark 2.4, as it needs data source v2 users to change the implementation intensitively, while they need to change again in next release.

## How was this patch tested?

existing tests

Author: Wenchen Fan <[email protected]>

Closes #22388 from cloud-fan/revert.
@gatorsmile
Copy link
Member

Unable to merge to master. @cloud-fan Could you submit another PR to the master branch?

@dongjoon-hyun
Copy link
Member

Ur, @gatorsmile . Why do we need to revert this from master?

@cloud-fan
Copy link
Contributor Author

As we discussed in the dev list, we only want to revert it from 2.4. I'm closing it now.

@cloud-fan cloud-fan closed this Sep 13, 2018
@rdblue
Copy link
Contributor

rdblue commented Sep 17, 2018

Thanks for doing this, @cloud-fan! Sorry I'm late to reply, I was at Strata all last week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants