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

#245 Add the ability to query REST endpoints from Reader module #297

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

benedeki
Copy link
Contributor

@benedeki benedeki commented Nov 1, 2024

  • Created GenericServerConnection offering the ability to query the Atum server
  • Several implementations of the generic connection offered, supporting different Asynchronous models
  • README.md updated to instruct how to use the Reader module and it's optional dependencies
  • The key method is GenericServerConnection.getQuery
  • ❓Open question: the result type of GenericServerConnection.getQuery, particularly the error case

Closes #245

Depends on #300

Release Notes:

  • Reader module now supports connection to Atum server.

@benedeki benedeki added the work in progress Work on this item is not yet finished (mainly intended for PRs) label Nov 1, 2024
@benedeki benedeki self-assigned this Nov 1, 2024
Copy link

github-actions bot commented Nov 1, 2024

JaCoCo model module code coverage report - scala 2.13.11

Overall Project 61.27% -35.88% 🍏
Files changed 66.75%

File Coverage
JsonSyntaxExtensions.scala 94.34% -5.66% 🍏
ErrorResponse.scala 89.66%
basic.scala 47.17%

Copy link

github-actions bot commented Nov 1, 2024

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project 78.2% 🍏
Files changed 100% 🍏

File Coverage
AtumAgent.scala 97.13% 🍏
AtumContext.scala 91.79% 🍏

Copy link

github-actions bot commented Nov 1, 2024

JaCoCo reader module code coverage report - scala 2.13.11

Overall Project 88.71% 🍏
Files changed 40.19%

File Coverage
PartitioningReader.scala 100%
Reader.scala 100% -14.75% 🍏
ReaderWithPartitioningId.scala 100% 🍏
io.scala 100% 🍏
future.scala 100% 🍏
ServerConfig.scala 88.24%
FlowReader.scala 80% -20% 🍏
RequestResult.scala 12.5%
zio.scala 0%

Copy link

github-actions bot commented Nov 1, 2024

JaCoCo server module code coverage report - scala 2.13.11

Overall Project 68.89% 🍏

There is no coverage information present for the Files changed


protected def executeRequest[R](request: RequestT[Identity, RequestResult[R], Any]): F[Response[RequestResult[R]]]

def getQuery[R: Decoder](endpointUri: String, params: Map[String, String] = Map.empty): F[RequestResult[R]] = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about the return type of this key function. Particularly the error case. But it can be relatively easily adjusted in future.

@benedeki benedeki marked this pull request as ready for review November 6, 2024 12:12
val expected: Unit = ()
for {
result <- connection.close()
} yield assertTrue(result == expected)
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are plenly of built-in assertions in ZIO.
https://zio.dev/reference/test/assertions/

assertZIO(connection.close())(isUnit)

@@ -18,6 +18,8 @@ package za.co.absa.atum.reader

import org.scalatest.funsuite.AnyFunSuiteLike

import za.co.absa.atum.reader.server.future.ArmeriaServerConnection.serverConnection

class FlowReaderUnitTests extends AnyFunSuiteLike {
test("foo") {
val expected = new FlowReader().foo()
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is actual value, the expected one is "bar" (it's the other way around)

@@ -18,6 +18,8 @@ package za.co.absa.atum.reader

import org.scalatest.funsuite.AnyFunSuiteLike

import za.co.absa.atum.reader.server.future.ArmeriaServerConnection.serverConnection

class PartitioningReaderUnitTests extends AnyFunSuiteLike {
test("foo") {
val expected = new PartitioningReader().foo()
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is actual value, the expected one is "bar" (it's the other way around)

Copy link
Collaborator

@salamonpavel salamonpavel left a comment

Choose a reason for hiding this comment

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

Tests of intended usage of the library are missing. It would be useful to implement one actual operation (read some Atum data with GET call) against mocked http server. And then for each flavor of "Connection" demonstrate intended usage E2E.

@benedeki benedeki added the dependent The item depends on some other open item (Issue or PR) label Nov 21, 2024
@benedeki benedeki removed the dependent The item depends on some other open item (Issue or PR) label Nov 22, 2024
@benedeki
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress Work on this item is not yet finished (mainly intended for PRs)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the ability to query REST endpoints from Reader module
2 participants