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

response envelope #199

Merged
merged 31 commits into from
Jun 13, 2024
Merged

response envelope #199

merged 31 commits into from
Jun 13, 2024

Conversation

salamonpavel
Copy link
Collaborator

@salamonpavel salamonpavel commented May 22, 2024

Introduces envelopes for API responses.
Closes #197

Release notes:

  • Introduces response envelopes providing additional metadata (requestId) for endpoints of version 2 (v2).
  • Exposes initial createPartitioningIfNotExists and createCheckpoint rest api endpoints in both existing versions (v1, v2).

@salamonpavel salamonpavel self-assigned this May 22, 2024
Copy link

github-actions bot commented May 22, 2024

JaCoCo server module code coverage report - scala 2.13.11

Overall Project 73.3% -21.23% 🍏
Files changed 55.77%

File Coverage
SuccessResponse.scala 100% -877.27%
BaseController.scala 100% 🍏
PartitioningControllerImpl.scala 88.89% -11.11% 🍏
CheckpointControllerImpl.scala 76% -76%
ErrorResponse.scala 69.39% -806.12%
PlayJsonImplicits.scala 68.66% 🍏

@salamonpavel salamonpavel marked this pull request as ready for review May 22, 2024 09:15
…e-envelope

# Conflicts:
#	server/src/test/scala/za/co/absa/atum/server/api/controller/CheckpointControllerIntegrationTests.scala
apiV1.post
.in(CreateCheckpoint)
.in(jsonBody[CheckpointDTO])
.out(statusCode(StatusCode.Created))
.out(jsonBody[CheckpointDTO])
.out(jsonBody[SingleSuccessResponse[CheckpointDTO]])
Copy link
Collaborator

Choose a reason for hiding this comment

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

actually I don't know if we want to do this - for potential compatibility reasons

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's talk about it on the internal MS chat

TebaleloS
TebaleloS previously approved these changes May 27, 2024
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.

[1] Pulled
[1] Tested
[1] Compiled

As much as I have approved this, can we please wait for David's PR about fixing the cross-build to be merged

Copy link

github-actions bot commented May 27, 2024

JaCoCo model module code coverage report - scala 2.13.11

Overall Project 60.76% 🍏

There is no coverage information present for the Files changed

Copy link

github-actions bot commented May 27, 2024

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project 86.35% -0.6% 🍏
Files changed 87.5% 🍏

File Coverage
HttpDispatcher.scala 47.44% -3.85% 🍏

@lsulak
Copy link
Collaborator

lsulak commented May 28, 2024

So probably the only thing now we have to do (after this PR, I'd say) is to make the agent compatible with v2 APIs. And also using circe for SerDe fully/everywhere, instead of the current approach.

@salamonpavel
Copy link
Collaborator Author

Release notes

  • Introduces response envelopes providing additional metadata (requestId) for endpoints of version 2 (v2).
  • Exposes initial createPartitioningIfNotExists and createCheckpoint rest api endpoints in both existing versions (v1, v2).

lsulak
lsulak previously approved these changes Jun 12, 2024
# Conflicts:
#	server/src/main/scala/za/co/absa/atum/server/api/controller/CheckpointController.scala
#	server/src/main/scala/za/co/absa/atum/server/api/controller/PartitioningController.scala
#	server/src/main/scala/za/co/absa/atum/server/api/controller/PartitioningControllerImpl.scala
#	server/src/main/scala/za/co/absa/atum/server/api/http/BaseEndpoints.scala
#	server/src/main/scala/za/co/absa/atum/server/api/http/Endpoints.scala
#	server/src/main/scala/za/co/absa/atum/server/api/http/Routes.scala
#	server/src/main/scala/za/co/absa/atum/server/model/PlayJsonImplicits.scala
@salamonpavel salamonpavel merged commit 628cf92 into master Jun 13, 2024
6 of 7 checks passed
@salamonpavel salamonpavel deleted the feature/197-response-envelope branch June 13, 2024 10:51
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.

Envelope for return types
3 participants