Feature/348 create checkpoint v2 #407
Conversation
…8-create-checkpoint-v2 # Conflicts: # agent/src/main/scala/za/co/absa/atum/agent/dispatcher/HttpDispatcher.scala
salamonpavel
requested review from
ABLL526,
MatloaItumeleng,
Zejnilovic,
benedeki,
dk1844 and
lsulak
as code owners
agent/src/main/scala/za/co/absa/atum/agent/dispatcher/ConsoleDispatcher.scala
Show resolved
Hide resolved
| val parentPartitioningIdOpt = partitioning.parentPartitioning.map(getPartitioningId) | ||
| val partitioningWithIdOpt = getPartitioning(partitioning.partitioning) | ||
|
|
||
|
|
| .as[SingleSuccessResponse[AdditionalDataDTO.Data]] | ||
| .data | ||
| .map(item => item._1 -> item._2.map(_.value)) | ||
|
|
|
|
||
| // Could be different from the one that was submitted. Replacing, just to have the most fresh copy possible. | ||
| this.additionalData = retrievedAD.data.map { case (k, v) => (k, v.map(_.value)) } | ||
| this.additionalData = retrievedAD.data.map { case (k, v) => (k, v.map(_.value)) }.toMap |
|
|
||
| import java.time.ZonedDateTime | ||
|
|
||
| @Ignore // has to be run with a real server |
There was a problem hiding this comment.
if it's so, then it is no longer a unit test. If we want to have unit tests here in this file, then we have to mock
There was a problem hiding this comment.
what changed? It used to be runnable 'locally' without the server, am I mistaken?
| val flywayUrl = s"jdbc:postgresql://$host:$port/$database" | ||
| val flywayUser = "postgres" | ||
| val flywayPassword = "postgres" | ||
| val flywayPassword = "changeme" |
There was a problem hiding this comment.
I see that you made this change on all places, what's the reason?
| .in(V2Paths.Partitionings / path[Long]("partitioningId") / V2Paths.AdditionalData) | ||
| .out(statusCode(StatusCode.Ok)) | ||
| .out(jsonBody[SingleSuccessResponse[AdditionalDataDTO.Data]]) | ||
| .out(jsonBody[MultiSuccessResponse[AdditionalDataItemV2DTO]]) |
There was a problem hiding this comment.
On one hand, I liked the SingleSuccessResponse[AdditionalDataDTO.Data] because it 'restricts' the duplication of same AD key, by the 'contract' it's immediatelly & explicitly obvious.
On another hand, I like MultiSuccessResponse[AdditionalDataItemV2DTO]] because it is more 'explicit' and precise on what is key, author, and value - and can be extended easily. Map of X and Y cannot be as easily.
So let's carry on with the implementation. It's same as for Measures. But it's important to know that we changed the contract here.
| import io.circe.{Decoder, Encoder} | ||
| import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder} | ||
|
|
||
| case class AdditionalDataItemV2DTO( |
There was a problem hiding this comment.
Ok, this structure makes sense to me.
But when I look at model/ now, it's very ... rich. I think that AdditionalDataDTO is only used for AD Update. What do you think about renaming it to UpdateAdditionalDataDTO ?
Please double check.
There was a problem hiding this comment.
Update: I noticed 1 more place where it's used, in Reader, in getAdditionalData - but I like that your structure AdditionalDataItemV2DTO more, and luckily we are not using this in UU project.
| */ | ||
| type AtumPartitions = ListMap[String, String] | ||
| type AdditionalData = Map[String, Option[String]] | ||
| type AdditionalData = Map[String, Option[String]] // why did we decide for the optionality of the value here? |
There was a problem hiding this comment.
IIRC AD value can be empty in general, we didn't want to restrict our users. It can be a prepared field, to be filled in later, or a part of patterns (we so far haven't really implemented / talked to that much)
Release notes:
saveCheckpointoperation inHttpDispatcheruses v2 endpointsCloses #348