Use error message type from spark commons: 2170 #2177

TebaleloS · 2023-02-24T12:49:23Z

Changes

Upgraded the spark-common version from 0.4.0 to 0.5.0
Removed ErrorMessage case class
Removed Mapping case class
Removed errorColumnName property
Renamed Enceladus ErrorMessage file and the Object to EnceladusErrorMessage
Replaced Enceladus ErrorMerssage with spark-common ErrorMessages
Closes Use ErrorMessage type from spark-commons #2170

…rorMessages and Mapping

benedeki

Seems class ErrorMessageFactory can be safely deleted too.

Does not build 😞 Several files still refer to old classes.

benedeki · 2023-02-24T22:29:05Z

...n/scala/za/co/absa/enceladus/plugins/builtin/errorsender/mq/KafkaErrorSenderPluginImpl.scala

 import za.co.absa.enceladus.utils.modules._
 import za.co.absa.spark.commons.implicits.StructTypeImplicits.StructTypeEnhancements
 import za.co.absa.abris.avro.functions.to_avro
 import za.co.absa.abris.config.{AbrisConfig, ToAvroConfig}
 import za.co.absa.enceladus.plugins.builtin.errorsender.mq.kafka.KafkaErrorSenderPlugin.{avroKeySchemaRegistryConfig, avroValueSchemaRegistryConfig, registerSchemas}
+import za.co.absa.enceladus.utils.error.EnceladusErrorMessage.ErrorCodes


Minor:
Some unused imports.

benedeki · 2023-02-24T22:56:59Z

spark-jobs/src/main/scala/za/co/absa/enceladus/conformance/interpreter/DynamicInterpreter.scala

@@ -28,19 +28,18 @@ import za.co.absa.enceladus.conformance.config.ConformanceConfigParser
 import za.co.absa.enceladus.conformance.datasource.PartitioningUtils
 import za.co.absa.enceladus.conformance.interpreter.rules._
 import za.co.absa.enceladus.conformance.interpreter.rules.custom.CustomConformanceRule
-import za.co.absa.enceladus.conformance.interpreter.rules.mapping.{MappingRuleInterpreter, MappingRuleInterpreterBroadcast,
-  MappingRuleInterpreterGroupExplode}
+import za.co.absa.enceladus.conformance.interpreter.rules.mapping.{MappingRuleInterpreter, MappingRuleInterpreterBroadcast, MappingRuleInterpreterGroupExplode}


Really minor:
Line is over 140 character long.

I'm not sure about what I should do here, or should I just import everything from maping e.g. za.co.absa.enceladus.conformance.interpreter.rules.mapping._?

I believe that multi-line imports should work:

import za.co.absa.enceladus.conformance.interpreter.rules.mapping.{ MappingRuleInterpreter, MappingRuleInterpreterBroadcast, MappingRuleInterpreterGroupExplode }

that can be one way how to resolve this

Yeah it does work, but gives a warning. But I have refactored it now.

benedeki · 2023-02-24T23:14:41Z

...bsa/enceladus/conformance/interpreter/rules/mapping/MappingRuleInterpreterGroupExplode.scala

@@ -25,6 +25,7 @@ import za.co.absa.enceladus.model.conformanceRule.{ConformanceRule, MappingConfo
 import za.co.absa.enceladus.model.{Dataset => ConfDataset}
 import za.co.absa.enceladus.utils.error._


lsulak

The changes looks good to me, but the code still won't compile because there are some places where it fails on wrong imports, as David said before.

Once it's fixed, I can quickly test it.

This reverts commit 8d4b3c6.

…ErrorMessage-type-from-spark-commons

lsulak · 2023-03-02T17:04:21Z

...n/scala/za/co/absa/enceladus/plugins/builtin/errorsender/mq/KafkaErrorSenderPluginImpl.scala

 import za.co.absa.enceladus.utils.modules._
 import za.co.absa.spark.commons.implicits.StructTypeImplicits.StructTypeEnhancements
 import za.co.absa.abris.avro.functions.to_avro
-import za.co.absa.abris.config.{AbrisConfig, ToAvroConfig}
+import za.co.absa.abris.config.{ToAvroConfig}
 import za.co.absa.enceladus.plugins.builtin.errorsender.mq.kafka.KafkaErrorSenderPlugin.{avroKeySchemaRegistryConfig, avroValueSchemaRegistryConfig, registerSchemas}


this line has 166 chars

...n/scala/za/co/absa/enceladus/plugins/builtin/errorsender/mq/KafkaErrorSenderPluginImpl.scala

lsulak · 2023-03-02T17:07:01Z

spark-jobs/src/test/scala/za/co/absa/enceladus/standardization/StandardizationRerunSuite.scala

@@ -47,6 +47,7 @@ class StandardizationRerunSuite extends FixtureAnyFunSuite with TZNormalizedSpar
  private val tmpFilePrefix = "test-input-"
  private val tmpFileSuffix = ".csv"

+


unnecessary empty line

lsulak · 2023-03-02T17:07:25Z

utils/src/main/scala/za/co/absa/enceladus/utils/error/EnceladusErrorMessage.scala

@@ -29,13 +30,13 @@ import za.co.absa.standardization.config.DefaultErrorCodesConfig
 * @param rawValues - Sequence of raw values (which are the potential culprits of the error)
 * @param mappings - Sequence of Mappings i.e Mapping Table Column -> Equivalent Mapped Dataset column
 */
-case class ErrorMessage(errType: String, errCode: String, errMsg: String, errCol: String, rawValues: Seq[String], mappings: Seq[Mapping] = Seq())
-case class Mapping(mappingTableColumn: String, mappedDatasetColumn: String)
+//case class ErrorMessage(errType: String, errCode: String, errMsg: String, errCol: String, rawValues: Seq[String], mappings: Seq[Mapping] = Seq())


dead code here and below

lsulak · 2023-03-02T17:07:51Z

utils/src/test/scala/za/co/absa/enceladus/utils/broadcast/BroadcastUtilsSuite.scala

@@ -18,8 +18,9 @@ package za.co.absa.enceladus.utils.broadcast
 import org.apache.spark.sql.functions._
 import org.apache.spark.sql.{DataFrame, Row}
 import org.scalatest.wordspec.AnyWordSpec
-import za.co.absa.enceladus.utils.error.Mapping
+import za.co.absa.enceladus.utils.error.EnceladusErrorMessage


this is not used

I don't have that file in my local branch.

how's that possible, did you remove it? What git status tells you?

lsulak

Looks good, I found some minor issues, most of them just formatting.

code reviewed
pulled
built
ran tests

…iltin/errorsender/mq/KafkaErrorSenderPluginImpl.scala Co-authored-by: Ladislav Sulak <[email protected]>

sonarcloud · 2023-03-03T10:43:02Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.0% Duplication

benedeki · 2023-04-18T09:00:01Z

I am sorry, I am afraid this PR become obsolete with ErrorHandling introduction. I suggest closing it, so as the ticket without merging. And deleting the branch.

TebaleloS added 3 commits February 24, 2023 11:34

Fixes #2170 - Replaced ErrorMessages and Mapping with spark-common Er…

eff8ef0

…rorMessages and Mapping

Fixes #2170 - Refactored some errorMessages imports

133bf0a

Fixes #2170 - Removed irrelevant doc

8d4b3c6

TebaleloS marked this pull request as ready for review February 24, 2023 16:07

TebaleloS requested review from benedeki, dk1844, lsulak and jakipatryk as code owners February 24, 2023 16:07

benedeki requested changes Feb 24, 2023

View reviewed changes

lsulak reviewed Feb 27, 2023

View reviewed changes

TebaleloS added 4 commits March 2, 2023 11:27

Revert "Fixes #2170 - Removed irrelevant doc"

af390cb

This reverts commit 8d4b3c6.

Removed ErrorMessageFactory

cfd669b

Merge remote-tracking branch 'origin/develop' into feature/#2170-Use-…

dba19a7

…ErrorMessage-type-from-spark-commons

Fixes #2170 - Removed unused imports and refactored complex imports

f70cf45

lsulak reviewed Mar 2, 2023

View reviewed changes

...n/scala/za/co/absa/enceladus/plugins/builtin/errorsender/mq/KafkaErrorSenderPluginImpl.scala Outdated Show resolved Hide resolved

lsulak reviewed Mar 2, 2023

View reviewed changes

Update plugins-builtin/src/main/scala/za/co/absa/enceladus/plugins/bu…

e2e7ddb

…iltin/errorsender/mq/KafkaErrorSenderPluginImpl.scala Co-authored-by: Ladislav Sulak <[email protected]>

benedeki added under discussion Requires consideration before a decision is made whether/how to implement work in progress Work on this item is not yet finished (mainly intended for PRs) labels Mar 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use error message type from spark commons: 2170 #2177

Use error message type from spark commons: 2170 #2177

TebaleloS commented Feb 24, 2023

benedeki left a comment

benedeki Feb 24, 2023

benedeki Feb 24, 2023

TebaleloS Mar 2, 2023

lsulak Mar 2, 2023 •

edited

Loading

TebaleloS Mar 2, 2023

benedeki Feb 24, 2023

lsulak left a comment

lsulak Mar 2, 2023

lsulak Mar 2, 2023

lsulak Mar 2, 2023

lsulak Mar 2, 2023

TebaleloS Mar 3, 2023

lsulak Apr 14, 2023

lsulak left a comment

sonarcloud bot commented Mar 3, 2023

benedeki commented Apr 18, 2023

		@@ -25,6 +25,7 @@ import za.co.absa.enceladus.model.conformanceRule.{ConformanceRule, MappingConfo
		import za.co.absa.enceladus.model.{Dataset => ConfDataset}
		import za.co.absa.enceladus.utils.error._

		@@ -47,6 +47,7 @@ class StandardizationRerunSuite extends FixtureAnyFunSuite with TZNormalizedSpar
		private val tmpFilePrefix = "test-input-"
		private val tmpFileSuffix = ".csv"

Use error message type from spark commons: 2170 #2177

Are you sure you want to change the base?

Use error message type from spark commons: 2170 #2177

Conversation

TebaleloS commented Feb 24, 2023

Changes

benedeki 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

lsulak Mar 2, 2023 • edited Loading

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

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

sonarcloud bot commented Mar 3, 2023

benedeki commented Apr 18, 2023

lsulak Mar 2, 2023 •

edited

Loading