Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upConversation
rossabaker
added
the
don't merge yet
label
Sep 22, 2018
rossabaker
reviewed
Sep 22, 2018
Note that almost every middleware is expressed in terms of client.run on the wrapped client. That means that even if we do override a method for efficiency, the override will be lost as soon as it's wrapped. This may be an argument for the original final class Client[F](run: Request => F[Response[F]]) encoding.
| def go[A](r: Resource[F, A]): F[(A, F[Unit])] = r match { | ||
| case Resource.Allocate(r) => | ||
| r.map { case (a, release) => (a, release(ExitCase.Completed)) } | ||
| case Resource.Bind(r, f) => |
This comment has been minimized.
This comment has been minimized.
rossabaker
Sep 22, 2018
Member
I am sleepy and this feels dangerous. Please be extra careful reviewing this.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rossabaker
Sep 24, 2018
Member
It probably pays to go back and understand why Resource has this ADT instead of being a wrapper around Allocate's value, because we're basically reconstructing that.
(This is also a suggestion for myself.)
This comment has been minimized.
This comment has been minimized.
rossabaker
added some commits
Sep 22, 2018
This comment has been minimized.
This comment has been minimized.
|
I have not tried the metrics, I have only compiled them. This is going to cause a merge conflict with #2094, so we need to give one of them the right of way. |
rossabaker
referenced this pull request
Sep 23, 2018
Merged
Refactor server creation into a resource #2104
rossabaker
modified the milestones:
0.19.0-M3,
0.19.0
Sep 23, 2018
This comment has been minimized.
This comment has been minimized.
|
Similar to, and supersedes, #1772. |
rossabaker
requested a review
from
ChristopherDavenport
Sep 24, 2018
rossabaker
removed
the
don't merge yet
label
Sep 24, 2018
rossabaker
added some commits
Sep 26, 2018
rossabaker
dismissed
aeons’s
stale review
Sep 26, 2018
Significantly changed toHttpApp
rossabaker
requested a review
from
SystemFw
Sep 26, 2018
rossabaker
modified the milestones:
0.19.0,
0.19.0-M3
Sep 26, 2018
| case (a, rel) => | ||
| stack match { | ||
| case Nil => F.pure(a -> release.guarantee(rel(ExitCase.Completed))) | ||
| case f0 :: xs => continue(f0(a), xs, release.guarantee(rel(ExitCase.Completed))) |
This comment has been minimized.
This comment has been minimized.
rossabaker
Sep 26, 2018
Member
We release in reverse order, so I think these guarantees are both backward? We don't yet have a test that exposes this.
aeons
merged commit 18a7520
into
http4s:master
Sep 27, 2018
added a commit
that referenced
this pull request
Sep 27, 2018
rossabaker commentedSep 22, 2018
DisposableResponseis goneClientis a trait with no concrete methodsDefaultClientimplements them all in terms of arun: Request[F] => Resource[F, Response[F]]shutdown: we create clients asResources.