-
Notifications
You must be signed in to change notification settings - Fork 1
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
get partitioning additional data v2 #252
get partitioning additional data v2 #252
Conversation
JaCoCo model module code coverage report - scala 2.13.11
|
JaCoCo agent module code coverage report - scala 2.13.11
|
JaCoCo server module code coverage report - scala 2.13.11
|
…ionalDataEndpointV2' into feature/227-getPartitioningAdditionalDataEndpointV2
Release notes:
|
database/src/main/postgres/runs/V1.9.6__get_partitioning_additional_data.sql
Outdated
Show resolved
Hide resolved
mapToSingleSuccessResponse( | ||
serviceCall[AdditionalDataDTO, AdditionalDataDTO]( | ||
partitioningService.getPartitioningAdditionalDataV2(partitioningId), | ||
identity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an idea, what about having identity
as the default parameter value? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, good idea. Applied.
server/src/main/scala/za/co/absa/atum/server/model/AdditionalDataItemFromDB.scala
Show resolved
Hide resolved
…ional_data.sql Co-authored-by: David Benedeki <[email protected]>
@@ -64,18 +64,36 @@ trait TestData { | |||
protected val measureFromDB1: MeasureFromDB = MeasureFromDB(Some("count1"), Some(Seq("col_A1", "col_B1"))) | |||
protected val measureFromDB2: MeasureFromDB = MeasureFromDB(Some("count2"), Some(Seq("col_A2", "col_B2"))) | |||
|
|||
// Additional Data | |||
protected val additionalDataDTO1: InitialAdditionalDataDTO = Map( | |||
// Initial Additional Data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think initial
is not descriptive enough (obviously not just here but also in the model's DTO) 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only a temporary name so that I can distinguish between newly created types and existing ones. The code related to InitialAdditionalData will be refactored to use the newly created classes. But it's going to be done in a separate PR in order to not affect scope of individual tickets.
LGTM, just one comment regarding the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- code reviewed
- pulled
- built
- run
Introduces GET endpoint for retrieving partitioning's additional data
Closes #227
Release notes: