AbsaOSS / atum-service

Apache License 2.0
5 stars 1 forks source link

Server endpoints v2 returning the checkpoins data of a partitioning: 190 #194

Closed TebaleloS closed 2 months ago

TebaleloS commented 3 months ago

closes #190

github-actions[bot] commented 3 months ago

JaCoCo server module code coverage report - scala 2.13.11

Overall Project 72.06% -23.65% :green_apple:
Files changed 73.64% :x:


File Coverage
BaseRepository.scala 100% :green_apple:
PartitioningRepositoryImpl.scala 100% :green_apple:
PartitioningControllerImpl.scala 100% :green_apple:
BaseController.scala 100% :green_apple:
PartitioningServiceImpl.scala 100% :green_apple:
BaseService.scala 100% :green_apple:
CheckpointFromDB.scala 85.83% -308.66% :x:
PlayJsonImplicits.scala 54.84% :green_apple:
GetPartitioningCheckpoints.scala 54.55% -56.36% :x:
github-actions[bot] commented 2 months ago

JaCoCo model module code coverage report - scala 2.13.11

Overall Project 60.76% -38.65% :green_apple:
Files changed 0% :x:


File Coverage
CheckpointQueryDTO.scala 0% -1385.71% :x:
github-actions[bot] commented 2 months ago

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project 86.3% :green_apple:

There is no coverage information present for the Files changed

lsulak commented 2 months ago

Looks good to me, review finished, just 1 minor thing, kind of consistency reasons; no other remarks outside of that, happy to approve soon!

lsulak commented 2 months ago

There are inconsistent namings. I would personally use getPartitioningCheckpoints everywhere (sometimes you use getPartitioningMeasures instead).

Absolutely, I support this idea. However, I think this PR (and few others) has been here for far too long, and we know that we already have some technical debt in endpoint/operation namings, plus we need to update the agent to support some of the missing operations (especially those for v2).

I'll approve now as I think that it's complete from my PoV.