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.

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] Use condition instead of errorClass in checkError() #3680

Merged
merged 5 commits into from
Sep 18, 2024

Conversation

MaxGekk
Copy link
Contributor

@MaxGekk MaxGekk commented Sep 16, 2024

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

In the PR, I propose to use the condition parameter instead of errorClass in calls of checkError because errorClass was renamed in Spark by the PR apache/spark#48027. This PR fixes compilation issues like:

[error]       checkError(
[error]       ^
[error] /home/runner/work/delta/delta/spark/src/test/scala/org/apache/spark/sql/delta/rowtracking/RowTrackingReadWriteSuite.scala:304:7: overloaded method checkError with alternatives:
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,sqlState: Option[String],parameters: Map[String,String],context: RowTrackingReadWriteSuite.this.ExpectedContext)Unit <and>

How was this patch tested?

By compiling locally.

Does this PR introduce any user-facing changes?

No. This makes changes in tests only.

@MaxGekk MaxGekk changed the title Use condition instead of errorClass in checkError() [Spark] Use condition instead of errorClass in checkError() Sep 16, 2024
@MaxGekk
Copy link
Contributor Author

MaxGekk commented Sep 16, 2024

@cloud-fan Could you have a look at the PR, please. It fixes compilation issues.

@cloud-fan
Copy link
Contributor

cloud-fan commented Sep 17, 2024

I think Delta master branch needs to cross-compile with both Spark master and 3.5 branches. We should create a shim layer for checkError

@MaxGekk
Copy link
Contributor Author

MaxGekk commented Sep 17, 2024

We should create a shim layer for checkError

@cloud-fan As an alternative solution, we could pass the first two arguments exception and condition (errorClass) by positions not by names. The rest args could be passed by names. I have double checked all checkError, they all starts from those two args:

  protected def checkError(
      exception: SparkThrowable,
      condition: String,

@MaxGekk
Copy link
Contributor Author

MaxGekk commented Sep 17, 2024

@cloud-fan Compilation has been fixed. The failed tests are not related to my changes, I hope.

@vkorukanti vkorukanti merged commit b2339cb into delta-io:master Sep 18, 2024
14 of 17 checks passed
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.

3 participants