Require an http4s client to provide more flexibility #373
Conversation
BenFradet
force-pushed
the
reuse-client
branch
2 times, most recently
from
f8534ae
to
c754d14
Compare
Codecov Report
@@ Coverage Diff @@
## master #373 +/- ##
==========================================
- Coverage 92.71% 92.65% -0.06%
==========================================
Files 23 23
Lines 535 531 -4
Branches 1 2 +1
==========================================
- Hits 496 492 -4
Misses 39 39
Continue to review full report at Codecov.
|
I haven't looked in detail but that might help with 47degrees/sbt-microsites#398 |
| ``` | ||
|
|
||
| `user1` in this case is a `IO[GHResponse[User]]`. | ||
|
|
||
| ### Using `F[_]: cats.effect.ConcurrentEffect` | ||
| ### Using `F[_]: cats.effect.Sync` |
There was a problem hiding this comment.
The actual context bound in the sample code below is : Async: ContextShift
There was a problem hiding this comment.
This is because of the chosen client, if it were BlazeClientBuilder it would be ConcurrentEffect. Our constraint is actually Sync: https://github.com/47degrees/github4s/pull/373/files#diff-9a35e1db598f031cb40b6fcaf7266225R24. Do you think this needs additional information / explanation?
There was a problem hiding this comment.
or I could write the example as:
def u1[F[_]: Sync](httpClient: Client[F]): F[GHResponse[User]] =
Github[F](httpClient, accessToken).users.get("juanpedromoreno")There was a problem hiding this comment.
which might be closer to what people end up writing anyway
| ) { | ||
|
|
||
| private lazy val module: GithubAPIs[F] = | ||
| new GithubAPIv3[F](accessToken, timeout.getOrElse(Duration(1000L, MILLISECONDS))) | ||
| new GithubAPIv3[F](client, accessToken) | ||
|
|
||
| lazy val users: Users[F] = module.users |
There was a problem hiding this comment.
Not related to this PR, but is there any reason for all this stuff to be lazy?
There was a problem hiding this comment.
I guess if you're using only one module / set of apis, the others don't get evaluated? 🤔
| .setThreadSub(validThreadId, true, false, headerUserAgent) | ||
| .unsafeRunSync() | ||
| val response = client | ||
| .use { client => |
There was a problem hiding this comment.
Maybe use a short name like c here, or rename the client in BaseIntegrationSpec to clientResource or something like, to avoid shadowing?
juanpedromoreno
added
the
breaking-change
This change avoids creating and tearing down a client on each call.
If this looks okay, I'll modify the docs as well.
cc @tovbinm