implement error handling that will filter the rows that have any error: #93 #94

TebaleloS · 2023-03-29T07:04:01Z

No description provided.

* 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

* headers fix

…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

…Errors

* `ErrorMessageSubmitWithoutColumn` changed from `case class` to `class` to allow inheritance * some PR comments addressed

* Fixed few minor things discovered by the UTs

…dling/implementations/ErrorHandlingFilterRowsWithErrors.scala Co-authored-by: David Benedeki <[email protected]>

…e-rows-that-have-any-error' of https://github.com/AbsaOSS/spark-commons into feature/93-Implement-error-handling-that-will-filter-the-rows-that-have-any-error

github-actions · 2023-05-05T14:21:33Z

JaCoCo code coverage report - scala:2.11 - spark:2.4

File	Coverage [95.97%]	🍏
ErrorMessageSubmitWithoutColumn.scala	100%	🍏
ErrorHandlingFilterRowsWithErrors.scala	100%	🍏
ColumnOrValue.scala	96.58%	🍏
ErrorHandling.scala	83.82%	🍏
ErrorMessageSubmit.scala	0%	❌

Total Project Coverage	82.07%	🍏

github-actions · 2023-05-05T14:21:34Z

JaCoCo code coverage report - scala:2.12 - spark:3.3

File	Coverage [97.5%]	🍏
ErrorMessageSubmitWithoutColumn.scala	100%	🍏
ErrorHandlingFilterRowsWithErrors.scala	100%	🍏
ErrorHandling.scala	100%	🍏
ColumnOrValue.scala	96.62%	🍏
ErrorMessageSubmit.scala	0%	❌

Total Project Coverage	85.42%	🍏

lsulak · 2023-05-06T11:54:11Z

.../co/absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrors.scala

+package za.co.absa.spark.commons.errorhandling.implementations
+
+import org.apache.spark.sql.{Column, DataFrame}
+import org.apache.spark.sql.functions.{coalesce, col, lit}


col is never used

lsulak · 2023-05-06T11:55:38Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+
+package za.co.absa.spark.commons.errorhandling.implementations
+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.functions.{col, column, length}


column is never used

lsulak · 2023-05-06T11:55:56Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.functions.{col, column, length}
+import org.scalatest.funsuite.AnyFunSuite
+import za.co.absa.spark.commons.errorhandling.ErrorMessage


ErrorMessage is never used

lsulak · 2023-05-06T11:56:17Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+  }
+
+  test("putError and putErrors does not group by together") {
+//    val errorMessageArray = ErrorMessageArray()


dead code, I believe it can be removed, unless you have some plans with this?

lsulak · 2023-05-06T11:57:15Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+    resultDf.as[ResultDfRecordType].collect().sortBy(_._1).toList
+  }
+
+  test("aggregateErrorColumns\" should \"return an empty list after error aggregation\"") {


why all these \" there?

lsulak · 2023-05-06T11:57:21Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+    assert(results == expectedResults)
+  }
+
+  test("aggregateErrorColumns\" should \"return  records whose don't have errors\"") {


why all these \" there?

Those I can actually remove, they are just for readability.

lsulak · 2023-05-06T11:59:09Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+    val e3 = ErrorHandlingFilterRowsWithErrors.putErrorToColumn(errorSubmitB)
+
+    val resultsDF = ErrorHandlingFilterRowsWithErrors.aggregateErrorColumns(srcDf)(e1, e2, e3)
+    resultsDF.show()


Can be removed, although I don't insist: https://softwareengineering.stackexchange.com/questions/415573/should-unit-tests-contain-print-statements

@benedeki what is your opinion on having prints in automated tests?

(this is present in a few tests below as well)

No prints (show is kind of print) in the final code.
I do use them occasionally for debugging etc during development, but should be removed before the actual merge.

Yah, I was also using them for debugging purposes. I actually forgot to remove them. But, yes they must be removed.

lsulak

This looks very good, @TebaleloS, I'm happy to approve once those tiny things I found are resolved. Great work!

benedeki · 2023-05-07T21:53:05Z

.../co/absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrors.scala

+ */
+
+object ErrorHandlingFilterRowsWithErrors extends ErrorHandlingCommon {


Suggested change

*/

object ErrorHandlingFilterRowsWithErrors extends ErrorHandlingCommon {

*/

object ErrorHandlingFilterRowsWithErrors extends ErrorHandlingCommon {

Minor: I believe ScalaDoc needs to be "attached" to the class/method. No empty lines between.

benedeki · 2023-05-07T21:59:40Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+    val e3 = ErrorHandlingFilterRowsWithErrors.putErrorToColumn(errorSubmitB)
+
+    val resultsDF = ErrorHandlingFilterRowsWithErrors.aggregateErrorColumns(srcDf)(e1, e2, e3)
+    resultsDF.show()


No prints (show is kind of print) in the final code.
I do use them occasionally for debugging etc during development, but should be removed before the actual merge.

benedeki · 2023-05-07T22:00:14Z

...absa/spark/commons/errorhandling/implementations/ErrorHandlingFilterRowsWithErrorsTest.scala

+    val resultsDf = ErrorHandlingFilterRowsWithErrors.putErrorsWithGrouping(srcDf)(
+      Seq(er1, er2, er3)
+    )
+    resultsDf.show()


Ditto, please remove.

benedeki

Nice documentation for the ErrorHandlingFilterRowsWithErrors class. Also good test coverage. 😉

code reviewed
pulled
built

benedeki and others added 11 commits March 7, 2023 04:29

* UT fix

de99046

* headers fix

* Work in progress

c50dd16

* Relatively big overwrite to use map instead of errCol and sequenc…

2db8c95

…e of raw values * `ErrorMessageSubmitJustErrorValue` class created to offer the ability to submit errors without source column but with error value

* Forgotten register function call

51b16f4

* line ends improved

4783fca

* ErrorMessageSubmits moved to submits sub-package

c8c4aa0

* `ErrorMessageSubmitWithoutColumn` changed from `case class` to `class` to allow inheritance * some PR comments addressed

Merge pull request #85 from AbsaOSS/feature/83-create-a-spike-for-err…

8e747e7

…or-handling-map-errcol Introducing Map for error columns and their values

* Added UTs for ColumnOrValue

64dc95e

* Fixed few minor things discovered by the UTs

Fixes #82 - Added logic to filter out rows with errors

5cdbc61

Fixes #82

af78cea

TebaleloS changed the base branch from master to feature/83-create-a-spike-for-error-handling March 29, 2023 07:11

TebaleloS added 3 commits March 29, 2023 11:54

Fixes #82 - created class

7c416b2

Fixes #93

1624f30

Fixes #93

2152467

benedeki force-pushed the feature/83-create-a-spike-for-error-handling branch from 64dc95e to c50dd16 Compare April 1, 2023 14:47

TebaleloS and others added 14 commits April 3, 2023 11:08

Fixes #93

e58ba18

Fixes #93 - Changed the logic for evaluate method

ebd844b

Fixes #93 - Added scala documentation for ErrorHandlingFilterRowsWith…

3efc1b5

…Errors

Fixes #93 - added ErrorHandlingFilterRowsWithErrors test file

38ad8a6

Fixes #93

fd89611

Fixes #93 - Object test progress.

24af555

Fixes #93 - Object test progress

98ccd6f

Fixes #93 - Object test progress

1b29d6c

Fixes #93 - Object test progress

72ef9af

* Forgotten register function call

a347ad8

* line ends improved

4359800

* ErrorMessageSubmits moved to submits sub-package

f361534

* `ErrorMessageSubmitWithoutColumn` changed from `case class` to `class` to allow inheritance * some PR comments addressed

* Added UTs for ColumnOrValue

63e7e3d

* Fixed few minor things discovered by the UTs

* Work in progress

bbf8d7e

TebaleloS requested review from Zejnilovic, dk1844 and lsulak as code owners April 28, 2023 17:10

TebaleloS and others added 5 commits April 28, 2023 19:11

Update spark-commons/src/main/scala/za/co/absa/spark/commons/errorhan…

985127d

…dling/implementations/ErrorHandlingFilterRowsWithErrors.scala Co-authored-by: David Benedeki <[email protected]>

Update spark-commons/src/main/scala/za/co/absa/spark/commons/errorhan…

3cc040f

…dling/implementations/ErrorHandlingFilterRowsWithErrors.scala Co-authored-by: David Benedeki <[email protected]>

#93 - Unit test work in progress

ba51fff

Merge branch 'feature/93-Implement-error-handling-that-will-filter-th…

eb908ce

…e-rows-that-have-any-error' of https://github.com/AbsaOSS/spark-commons into feature/93-Implement-error-handling-that-will-filter-the-rows-that-have-any-error

#93 - Unit test work in progress

4792657

benedeki mentioned this pull request May 2, 2023

#101: ErrorHandling documentation and fields renames #103

Merged

Base automatically changed from feature/83-create-a-spike-for-error-handling to master May 4, 2023 14:57

TebaleloS added 4 commits May 4, 2023 21:09

Fixes #93 conflicts

6dbe960

Fixes #93 conflicts

7d82796

Fixes #93

5380858

Fixes #93

4d8d7ea

Fixes #93

cbd9c23

lsulak reviewed May 6, 2023

View reviewed changes

benedeki reviewed May 7, 2023

View reviewed changes

Closes #93

467d353

benedeki approved these changes May 8, 2023

View reviewed changes

TebaleloS merged commit 279b4e5 into master May 8, 2023

TebaleloS deleted the feature/93-Implement-error-handling-that-will-filter-the-rows-that-have-any-error branch May 8, 2023 12:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement error handling that will filter the rows that have any error: #93 #94

implement error handling that will filter the rows that have any error: #93 #94

TebaleloS commented Mar 29, 2023

github-actions bot commented May 5, 2023 •

edited

Loading

github-actions bot commented May 5, 2023 •

edited

Loading

lsulak May 6, 2023

lsulak May 6, 2023

lsulak May 6, 2023

lsulak May 6, 2023

lsulak May 6, 2023

lsulak May 6, 2023

TebaleloS May 8, 2023

lsulak May 6, 2023

lsulak May 6, 2023

benedeki May 7, 2023

TebaleloS May 8, 2023

lsulak left a comment

benedeki May 7, 2023

benedeki May 7, 2023

benedeki May 7, 2023

benedeki left a comment

		*/

		object ErrorHandlingFilterRowsWithErrors extends ErrorHandlingCommon {

implement error handling that will filter the rows that have any error: #93 #94

implement error handling that will filter the rows that have any error: #93 #94

Conversation

TebaleloS commented Mar 29, 2023

github-actions bot commented May 5, 2023 • edited Loading

JaCoCo code coverage report - scala:2.11 - spark:2.4

github-actions bot commented May 5, 2023 • edited Loading

JaCoCo code coverage report - scala:2.12 - spark:3.3

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lsulak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benedeki left a comment

Choose a reason for hiding this comment

github-actions bot commented May 5, 2023 •

edited

Loading

github-actions bot commented May 5, 2023 •

edited

Loading