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

#105: Refactoring Measurement, from Agent side #104

Merged
merged 44 commits into from
Mar 8, 2024

Conversation

lsulak
Copy link
Collaborator

@lsulak lsulak commented Nov 1, 2023

Addressing Measurement and MeasurementResult - I tried to make it much more simpler and added some validations even. Now Measure contains only 2 attributes; the result itself is separated.

I improved the MeasurementBuilder and now the Measures and Measurements are Set but with some additional validation on uniqueness - i.e. a user can't define the same Measures against the same columns with different result values - it's not a valid case.

Closes #105

@lsulak lsulak changed the title Refactoring Measurement and a lot of stuff from Agent side Refactoring Measurement, from Agent side Nov 1, 2023
@lsulak lsulak self-assigned this Nov 1, 2023
@lsulak lsulak added the work in progress Work on this item is not yet finished (mainly intended for PRs) label Nov 1, 2023
@lsulak lsulak marked this pull request as draft November 1, 2023 11:57
Copy link

github-actions bot commented Nov 1, 2023

JaCoCo agent module code coverage report - spark:2 - scala 2.12.12

Build Failed

Copy link

github-actions bot commented Nov 1, 2023

JaCoCo server module code coverage report - scala 2.12.12

Build Failed

@lsulak lsulak removed the work in progress Work on this item is not yet finished (mainly intended for PRs) label Nov 1, 2023
@lsulak lsulak changed the title Refactoring Measurement, from Agent side #105: Refactoring Measurement, from Agent side Nov 1, 2023
@lsulak lsulak marked this pull request as ready for review November 2, 2023 10:07
@lsulak lsulak added the work in progress Work on this item is not yet finished (mainly intended for PRs) label Nov 2, 2023
Copy link
Collaborator

@TebaleloS TebaleloS left a comment

Choose a reason for hiding this comment

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

This looks good. The only problem is that it cannot compile successfully, it has a type mismatch on found : scala.collection.immutable.Set[String] [error] required: Seq[Any] [error] val measureNames = scalaIterableToSQLArray(values.measurements.map(_.measure.measureName))

@lsulak
Copy link
Collaborator Author

lsulak commented Nov 9, 2023

This looks good. The only problem is that it cannot compile successfully, it has a type mismatch on found : scala.collection.immutable.Set[String] [error] required: Seq[Any] [error] val measureNames = scalaIterableToSQLArray(values.measurements.map(_.measure.measureName))

Fixed, thanks, it's already in master now

Copy link

github-actions bot commented Nov 10, 2023

JaCoCo agent module code coverage report - spark:2 - scala 2.12.18

Build Failed

Copy link

github-actions bot commented Nov 10, 2023

JaCoCo server module code coverage report - scala 2.12.18

Build Failed

…-refactoring

# Conflicts:
#	agent/src/main/scala/za/co/absa/atum/agent/AtumContext.scala
#	agent/src/main/scala/za/co/absa/atum/agent/model/Measure.scala
#	agent/src/main/scala/za/co/absa/atum/agent/model/MeasurementBuilder.scala
#	model/src/main/scala/za/co/absa/atum/model/dto/CheckpointDTO.scala
…-refactoring

# Conflicts:
#	agent/src/main/scala/za/co/absa/atum/agent/AtumContext.scala
#	agent/src/main/scala/za/co/absa/atum/agent/exception/AtumAgentException.scala
#	agent/src/main/scala/za/co/absa/atum/agent/model/Measurement.scala
#	agent/src/test/scala/za/co/absa/atum/agent/model/MeasurementTest.scala
#	model/src/main/scala/za/co/absa/atum/model/dto/CheckpointDTO.scala
…-refactoring

# Conflicts:
#	agent/src/main/scala/za/co/absa/atum/agent/AtumAgent.scala
#	agent/src/main/scala/za/co/absa/atum/agent/dispatcher/HttpDispatcher.scala
#	server/src/main/scala/za/co/absa/atum/server/api/database/Runs.scala
@@ -64,7 +64,7 @@ class AtumAgent private[agent] () {
}

/**
* Provides an AtumContext given a `AtumPartitions` instance. Retrieves the data from AtumService API.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link

github-actions bot commented Feb 1, 2024

JaCoCo server module code coverage report - scala 2.13.11

Overall Project 61.33% -13.71%
Files changed 16.6%

File Coverage
PlayJsonImplicits.scala 30.18% -71.27%

Copy link

github-actions bot commented Feb 12, 2024

JaCoCo agent module code coverage report - scala 2.12.18

Overall Project NaN% NaN% 🍏

There is no coverage information present for the Files changed

Copy link

github-actions bot commented Feb 12, 2024

JaCoCo model module code coverage report - scala 2.12.18

Overall Project NaN% NaN% 🍏

There is no coverage information present for the Files changed

salamonpavel
salamonpavel previously approved these changes Feb 16, 2024
benedeki
benedeki previously approved these changes Feb 28, 2024
Copy link
Contributor

@benedeki benedeki left a comment

Choose a reason for hiding this comment

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

Let's merge it. A major improvement.

- name: Build and run tests
run: sbt ++${{matrix.scala}} test doc
run: sbt "project server" ++${{matrix.scala}} test doc
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't missing the db module (tests)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

… Java 11 uses different algorithm - JLS3 "Binary Compatibility" you can read more about it if interested; this PR addresses that and removes reflection completely from this SerDe
Copy link

github-actions bot commented Mar 5, 2024

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project 73.43% -48.98%
Files changed 52.57%

File Coverage
MeasureResult.scala 100% -260%
MeasurementBuilder.scala 94.74% -5.26% 🍏
AtumContext.scala 91.11% 🍏
Measure.scala 88.38% -67.84%
MeasuresBuilder.scala 83.87% -12.9% 🍏
AtumAgent.scala 77.44% 🍏
AtumAgentException.scala 64.71% -347.06%
HttpDispatcher.scala 0% -19.64%

Copy link

github-actions bot commented Mar 5, 2024

JaCoCo model module code coverage report - scala 2.13.11

Overall Project 60.65% -64.91%
Files changed 32.49%

File Coverage
CheckpointDTO.scala 100% -20.69%
MeasureResultDTO.scala 75.68% -475.68%
SerializationUtils.scala 47.91% -64.19%

Copy link
Contributor

@benedeki benedeki left a comment

Choose a reason for hiding this comment

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

Let's get it in 😉

@lsulak lsulak merged commit a7f3f64 into master Mar 8, 2024
8 of 10 checks passed
@lsulak lsulak deleted the feature/measurement-refactoring branch March 8, 2024 11:01
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.

Agent: Refactor Measurements
4 participants