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

#280: agent and server compatibility for Additional Data PATCH operation #283

Merged
merged 26 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a0e7045
#280: agent and server compatibility in AD
lsulak Oct 1, 2024
6f10138
#280: agent and server compatibility in AD
lsulak Oct 1, 2024
550d557
Merge branch 'master' into feature/280-adopt-agent-to-server-in-v0-3-0
lsulak Oct 1, 2024
978ea21
#280: removing unnecessary DTOs and refactoring
lsulak Oct 1, 2024
74f3837
#280: unit test of DTO Base 64 encoder
lsulak Oct 1, 2024
ae9875d
#280: replacing HTTP backend, 'HttpURLConnectionBackend' doesn't supp…
lsulak Oct 2, 2024
242452d
#280: adding E2E integration test using Balta
lsulak Oct 2, 2024
88ec23d
#280: Adding e2e test - not part of CI yet!
lsulak Oct 2, 2024
4bdeb99
#280: adding missing licence headers
lsulak Oct 2, 2024
d01aaea
#280: changing back - unit tests would be otherwise working with real…
lsulak Oct 2, 2024
2534a6b
#280: changing back
lsulak Oct 2, 2024
a3519c9
#280: refactoring
lsulak Oct 2, 2024
b479c62
#280: ignoring e2e test for now
lsulak Oct 2, 2024
204f1b1
#280: test file must be with this suffix
lsulak Oct 2, 2024
fd2c52f
#280: AgentWithServerIntegrationTests should run against local servic…
lsulak Oct 2, 2024
10ff455
#280: ignoring e2e test for now
lsulak Oct 2, 2024
caae842
#280: ignoring e2e test for now - proper way
lsulak Oct 2, 2024
068dc66
#280: adding exclusions to jacoco coverage ignore - these 2 dispatche…
lsulak Oct 2, 2024
c785a49
#280: adding exclusions to jacoco coverage ignore - envelope DTOs are…
lsulak Oct 2, 2024
95cbe0a
#280: post-review improvements
lsulak Oct 2, 2024
4ead4dc
#280: syndrome under control
lsulak Oct 2, 2024
0cdc465
#280: actually using Atum HTTP Dispatcher config
lsulak Oct 2, 2024
9b344b5
#280: post-review changes, refactoring simplification, removing old u…
lsulak Oct 3, 2024
7d856d3
Merge remote-tracking branch 'origin/master' into feature/280-adopt-a…
lsulak Oct 3, 2024
522ad56
#280: post-merge conflict resolutions
lsulak Oct 3, 2024
2b20f17
#280: post-merge conflict resolutions
lsulak Oct 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class AtumContext private[agent] (
*
* @return the current additional data
*/
def currentAdditionalData: InitialAdditionalDataDTO = {
def currentAdditionalData: AdditionalData = {
additionalData
}

Expand Down Expand Up @@ -199,7 +199,7 @@ class AtumContext private[agent] (
atumPartitions: AtumPartitions = atumPartitions,
agent: AtumAgent = agent,
measures: Set[AtumMeasure] = measures,
additionalData: InitialAdditionalDataDTO = additionalData
additionalData: AdditionalData = additionalData
): AtumContext = {
new AtumContext(atumPartitions, agent, measures, additionalData)
}
Expand All @@ -210,7 +210,7 @@ object AtumContext {
* Type alias for Atum partitions.
*/
type AtumPartitions = ListMap[String, String]
type AdditionalData = InitialAdditionalDataDTO
type AdditionalData = Map[String, Option[String]]

/**
* Object contains helper methods to work with Atum partitions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ import sttp.client3._
import sttp.model.Uri
import sttp.client3.okhttp.OkHttpSyncBackend
import za.co.absa.atum.agent.exception.AtumAgentException.HttpException
import za.co.absa.atum.model.dto
import za.co.absa.atum.model.dto._
import za.co.absa.atum.model.envelopes.SuccessResponse.SingleSuccessResponse
import za.co.absa.atum.model.utils.DTOBase64Encoder.encodeDTOToBase64
import za.co.absa.atum.model.utils.JsonSyntaxExtensions._


Expand Down Expand Up @@ -63,7 +61,7 @@ class HttpDispatcher(config: Config) extends Dispatcher(config) with Logging {
*/

private[dispatcher] def getPartitioningId(partitioning: PartitioningDTO): Long = {
val encodedPartitioning = encodeDTOToBase64(partitioning)
val encodedPartitioning = partitioning.asBase64EncodedJsonString
val request = commonAtumRequest.get(getPartitioningIdEndpoint.addParam("partitioning", encodedPartitioning))

val response = backend.send(request)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package za.co.absa.atum.database.runs
import za.co.absa.balta.DBTestSuite
import za.co.absa.balta.classes.JsonBString

class GetPartitioningAdditionalDataIntegrationTests extends DBTestSuite{
class GetPartitioningAdditionalDataIntegrationTests extends DBTestSuite {

private val fncGetPartitioningAdditionalData = "runs.get_partitioning_additional_data"

Expand Down Expand Up @@ -88,19 +88,21 @@ class GetPartitioningAdditionalDataIntegrationTests extends DBTestSuite{
)

function(fncGetPartitioningAdditionalData)
.setParam("i_partitioning", partitioning1)
.setParam("i_partitioning_id", fkPartitioning1)
Copy link
Contributor

Choose a reason for hiding this comment

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

You know, that you don't have to name the params, you can put them there positionally? Especially in one param functions, it's easier

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, I know, but I find it nicer - more readable if it's named, especially if there are more functions; but for 1-2 params I think you are right, can be ommited

.execute { queryResult =>
val results = queryResult.next()
assert(results.getInt("status").contains(11))
assert(results.getString("status_text").contains("OK"))
assert(results.getString("ad_name").contains("ad_1"))
assert(results.getString("ad_value").contains("This is the additional data for Joseph"))
assert(results.getString("ad_author").contains("Joseph"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Really need to switch to Balta 0.3, so much tidier 😉


val results2 = queryResult.next()
assert(results2.getInt("status").contains(11))
assert(results2.getString("status_text").contains("OK"))
assert(results2.getString("ad_name").contains("ad_2"))
assert(results2.getString("ad_value").contains("This is the additional data for Joseph"))
assert(results2.getString("ad_author").contains("Joseph"))

assert(!queryResult.hasNext)
}
Expand All @@ -124,8 +126,11 @@ class GetPartitioningAdditionalDataIntegrationTests extends DBTestSuite{
val fkPartitioning: Long = table("runs.partitionings").fieldValue("partitioning", partitioning2, "id_partitioning").get.get

function(fncGetPartitioningAdditionalData)
.setParam("i_partitioning", partitioning2)
.setParam("i_partitioning_id", fkPartitioning)
.execute { queryResult =>
val result = queryResult.next()
assert(result.getInt("status").contains(16))
assert(result.getString("status_text").contains("No additional data found"))
assert(!queryResult.hasNext)
}

Expand All @@ -135,20 +140,8 @@ class GetPartitioningAdditionalDataIntegrationTests extends DBTestSuite{
}

test("Get partitioning additional data should return error status code on non existing partitioning") {
val partitioning = JsonBString(
"""
|{
| "version": 1,
| "keys": ["key1"],
| "keysToValuesMap": {
| "key1": "value1"
| }
|}
|""".stripMargin
)

function(fncGetPartitioningAdditionalData)
.setParam("i_partitioning", partitioning)
.setParam("i_partitioning_id", 0L)
.execute { queryResult =>
val results = queryResult.next()
assert(results.getInt("status").contains(41))
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import io.circe._
case class AtumContextDTO(
partitioning: PartitioningDTO,
measures: Set[MeasureDTO] = Set.empty,
additionalData: InitialAdditionalDataDTO = Map.empty
additionalData: Map[String, Option[String]] = Map.empty
)

object AtumContextDTO {
Expand Down
11 changes: 0 additions & 11 deletions model/src/main/scala/za/co/absa/atum/model/dto/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,7 @@

package za.co.absa.atum.model

import io.circe._

package object dto {
type PartitioningDTO = Seq[PartitionDTO]
type InitialAdditionalDataDTO = Map[String, Option[String]]

// Todo. This implicit definition should not be defined here, so it is to be addressed in Ticket #221
// Implicit encoders and decoders for AdditionalDataDTO
implicit val decodeAdditionalDataDTO: Decoder[InitialAdditionalDataDTO] = Decoder.decodeMap[String, Option[String]]
implicit val encodeAdditionalDataDTO: Encoder[InitialAdditionalDataDTO] = Encoder.encodeMap[String, Option[String]]

// Implicit encoders and decoders for PartitioningDTO
implicit val decodePartitioningDTO: Decoder[PartitioningDTO] = Decoder.decodeSeq[PartitionDTO]
implicit val encodePartitioningDTO: Encoder[PartitioningDTO] = Encoder.encodeSeq[PartitionDTO]
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package za.co.absa.atum.model.envelopes

case class StatusResponse(status: String, message: String)
case class StatusResponse private (status: String, message: String)

object StatusResponse {

Expand All @@ -25,7 +25,7 @@ object StatusResponse {
implicit val encoder: io.circe.Encoder[StatusResponse] = deriveEncoder
implicit val decoder: io.circe.Decoder[StatusResponse] = deriveDecoder

def up: StatusResponse = {
lazy val up: StatusResponse = {
StatusResponse(
status = "UP",
message = "Atum server is up and running"
Expand Down
Loading
Loading