-
Notifications
You must be signed in to change notification settings - Fork 0
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
#101: ErrorHandling documentation and fields renames #103
#101: ErrorHandling documentation and fields renames #103
Conversation
* new functions `null_col` and `call_udf` * `ErrorMessage` refactoring * `ErrorHandling` trait designed to serve as the interface for different implementations * Implement error handling by putting the info into column of `ErrorMessage` array * numerous support classes
…e of raw values * `ErrorMessageSubmitJustErrorValue` class created to offer the ability to submit errors without source column but with error value
* `ErrorMessageSubmitWithoutColumn` changed from `case class` to `class` to allow inheritance * some PR comments addressed
…or-handling-map-errcol Introducing Map for error columns and their values
* Fixed few minor things discovered by the UTs
…dling/implementations/ErrorMessageArray.scala Co-authored-by: Ladislav Sulak <[email protected]>
* merged `ErrorHandlingCommon` into `ErrorHandling` * renamed errCol / errCols => errSrcColName * renamed putErrorToColumn => createErrorAsColumn * renamed aggregateErrorColumns => applyErrorColumnsToDataFrame * renamed `EvaluateIntoErrorMessage` to `TransformIntoErrorMessage` * renamed `EvaluateViaUdf` to `TransformViaUdf` * some smaller parameters renamed * numerous ScalaDoc entries
...sa/spark/commons/errorhandling/implementations/submits/ErrorMessageSubmitOnMoreColumns.scala
Show resolved
Hide resolved
spark-commons/src/main/scala/za/co/absa/spark/commons/errorhandling/types/ErrorWhen.scala
Outdated
Show resolved
Hide resolved
* typo fix * enhanced information about partials
JaCoCo code coverage report - scala:2.12 - spark:3.3
|
...sa/spark/commons/errorhandling/implementations/submits/ErrorMessageSubmitWithoutColumn.scala
Outdated
Show resolved
Hide resolved
...park/commons/errorhandling/implementations/submits/ErrorMessageSubmitWithoutColumnTest.scala
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I reviewed it all, including git pull, sbt build, and I also ran tests. More than happy to approve once those minor findings are resolved
* documentation to `ErrorMessageArray` * `null_col` function description enhanced
...rc/main/scala/za/co/absa/spark/commons/errorhandling/implementations/ErrorMessageArray.scala
Outdated
Show resolved
Hide resolved
...park/commons/errorhandling/implementations/submits/ErrorMessageSubmitWithoutColumnTest.scala
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
- code reviewed
- pulled
- built
- ran tests
build.sbt
Outdated
lazy val commonJacocoExcludes: Seq[String] = Seq( | ||
"za.co.absa.spark.commons.adapters.CallUdfAdapter", | ||
"za.co.absa.spark.commons.adapters.TransformAdapter" | ||
// "za.co.absa.spark.commons.utils.JsonUtils*", // class and related objects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see dead code, is reserved for something or can removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still here (and 1 below)
build.sbt
Outdated
"za.co.absa.spark.commons.adapters.CallUdfAdapter", | ||
"za.co.absa.spark.commons.adapters.TransformAdapter" | ||
// "za.co.absa.spark.commons.utils.JsonUtils*", // class and related objects | ||
// "za.co.absa.spark.commons.utils.ExplodeTools" // class only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example with * is still hint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will actually change it to make it obvious it's a documentation and not dead code 😉
…d-fields-renames # Conflicts solved: # spark-commons/src/main/scala/za/co/absa/spark/commons/errorhandling/ErrorHandling.scala # spark-commons/src/main/scala/za/co/absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrors.scala # spark-commons/src/main/scala/za/co/absa/spark/commons/errorhandling/partials/ErrorHandlingCommon.scala # spark-commons/src/test/scala/za/co/absa/spark/commons/errorhandling/implementations/ErrorMessageArrayTest.scala
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, did the last round of review - pulled, built, ran tests, reviewed.
ErrorHandlingCommon
intoErrorHandling
EvaluateIntoErrorMessage
toTransformIntoErrorMessage
EvaluateViaUdf
toTransformViaUdf
Depends on #84Depends on #94Closes #101