Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Oct 18 15:46
    codecov[bot] commented #898
  • Oct 18 15:46
    julienrf synchronize #898
  • Oct 18 12:25
    codecov[bot] commented #929
  • Oct 18 12:16
    scala-steward opened #929
  • Oct 15 14:33
    codecov[bot] commented #918
  • Oct 15 14:31
    scala-steward synchronize #926
  • Oct 15 14:30
    scala-steward synchronize #918
  • Oct 15 14:30
    scala-steward synchronize #928
  • Oct 15 12:48
    julienrf commented #893
  • Oct 15 12:48
    julienrf commented #893
  • Oct 15 12:45
    julienrf closed #890
  • Oct 15 12:45
    julienrf commented #890
  • Oct 15 12:44

    julienrf on master

    Ujson: decoding floating infini… (compare)

  • Oct 15 12:44
    julienrf closed #893
  • Oct 15 12:36
    codecov[bot] commented #918
  • Oct 15 12:34
    codecov[bot] commented #928
  • Oct 15 12:27
    codecov[bot] commented #918
  • Oct 15 12:27
    scala-steward synchronize #918
  • Oct 15 12:27
    scala-steward synchronize #926
  • Oct 15 12:27
    scala-steward closed #892
Julien Richard-Foy
@julienrf
It’s for the case of the request entity is streamed, but not the response
Dominic Egger
@GrafBlutwurst
right so it can be assumed that by the time the stream hits implementedByEffect it has been reassembled and the stream only ever contains one element?
Julien Richard-Foy
@julienrf
Yes, that’s the problem we have with the current way of framing chunks
Dominic Egger
@GrafBlutwurst
right, now I understand the conversation much better.
Dominic Egger
@GrafBlutwurst
but that issue is not specific to the http4s implementation is it? it seems to be the same in akka where we get Source[Req, _]
Julien Richard-Foy
@julienrf
Yes
Well, we do the same assumptions on all the interpreters
It turns out that the akka-http interpreters work well with this assumption (chunks are not reframed)
same for play-server
Dominic Egger
@GrafBlutwurst
you wouldn't happen to have a log/dump of the particular errors you encountered due to reframing anywhere?
Julien Richard-Foy
@julienrf
No :( it seems that Travis CI does not have the logs of the tests either
but if you managed to get the tests pass, then it means that it works, no?
Dominic Egger
@GrafBlutwurst

weeeell. the server ones are but there's an exception that doesn't fail the test somehow. client tests are not passing. I just got them compiling. I suspect this bit from http4s docs might be pertinent though:

Next, we want streaming JSON. Because http4s is streaming in its bones, it relies on jawn for streaming JSON support. Most popular JSON libraries, including Circe, provide jawn support, as the code below shows. What’s left is to integrate jawn’s streaming parsing with fs2’s Stream. That’s done by jawn-fs2, which http4s’ Circe module depends on transitively.

I think you hit the nail on the head with what you wrote in the PR
I don't see a way to address this without baking in circe or at least jawn-fs2 into the http4s-client
Julien Richard-Foy
@julienrf
Ah, if that’s a problem we can create separate modules (e.g., http4s-streaming-server)
but I guess there is a high chance that most of the users of http4s also use the http4s-circe module?
Dominic Egger
@GrafBlutwurst
I would suspect most. I'll do some more tinkering to confirm if that's the issue
Dominic Egger
@GrafBlutwurst
hahaha I think we lazified ourselfs to death because we have an F[Stream[F,Resp]] within a Resource.use i just wrote some truly heretical code, but all tests are green. maybe someone smarter than I knows how to do this properly
Dominic Egger
@GrafBlutwurst
https://github.com/rivero-ag/endpoints4s/blob/http4s-chunked-encoding/http4s/client/src/main/scala/endpoints4s/http4s/client/ChunkedEntities.scala#L55 this is some truly cursed code but makes the tests green. If somebody has some good input on how to address this, I'd be very glad. I'm out for the day though
Dominic Egger
@GrafBlutwurst
PR is up to be looked at. It has a couple open points that I don't know how to tackle so I'd be extremely happy for some help
Julien Richard-Foy
@julienrf
@GrafBlutwurst great, I will have a look ASAP, thanks!
Dominic Egger
@GrafBlutwurst
thank you!
Julien Richard-Foy
@julienrf
@GrafBlutwurst I am not familiar with http4s but I have left a couple of comments, let me know what you think
Dominic Egger
@GrafBlutwurst
:+1: thanks!
Dominic Egger
@GrafBlutwurst
you were indeed right @julienrf the other streams need to be forced as well, it was missing tests. I'm currently adding them. Regarding the change of the Endpointsignature, how much of a problem is that gonna be for users since we're gonna break signature compatibility
Dominic Egger
@GrafBlutwurst
also changing the effect type to be Resource wrapped does work!
Julien Richard-Foy
@julienrf
I think it’s okay to introduce a breaking change in the http4s interpreters, if it brings useful features
So, which type did you change, and how, to get something that works?
Dominic Egger
@GrafBlutwurst

as you proposed I changed in http4s-client this definition

  //#endpoint-type
  type EffectResource[A] = Resource[Effect, A]
  type Endpoint[A, B] = Kleisli[EffectResource, A, B]
  //#endpoint-type

(for some reason it doesn't accept * hence the type alias indirection)

which then allowed me to no longer force streams in ChunkedEntitiesand rather consume them as intended on the tests
Julien Richard-Foy
@julienrf
Good! Do you think that type also makes sense for non-streaming requests?
Dominic Egger
@GrafBlutwurst
I think it generally makes sense, because we can't know on a level where we only have a type param Respwhether it's fine to consume the resource or not. If you depend lazily on the Response in any way shape or form you're gonna be bitten the same way
though I think you have a point that these usecases are probably pretty sparse
MR is updated
Julien Richard-Foy
@julienrf
Just an idea: instead of a type alias to Kleisli, we could define a proper type Endpoint[A, B] with a convenient operation for invoking a non-chunked endpoint:
trait Endpoint[A, B] {
  def lowLevelApply(a: A): Resource[Effect, B]
  def convenientApply(a: A): Effect[B] = ... 
}
Dominic Egger
@GrafBlutwurst
that is probably a good idea. Namingwise what do you think? perhaps .resourceand .use ?
Dominic Egger
@GrafBlutwurst
done
mhhh use might be a bad name.
Julien Richard-Foy
@julienrf
Maybe something with “consume” in it?
Not sure about “resource” as well as a name, maybe send, and sendAndConsume?
(I’m not sure when exactly the request is sent, maybe it’s not when we call these methods but later?)
Dominic Egger
@GrafBlutwurst
actually good question. I don't know if it's part of the use or the run
I adjusted it just now on the MR to send and sendAndConsume
mhh I would prefer to convey to a user that sendAndConsumeis potentially unsafe. I added a doc-comment for now
Julien Richard-Foy
@julienrf
We need to know whether use sends the request or run sends the request :)
Dominic Egger
@GrafBlutwurst
lemme do some digging
Dominic Egger
@GrafBlutwurst
So reading this (https://github.com/http4s/http4s/blob/main/blaze-client/src/main/scala/org/http4s/blaze/client/BlazeClient.scala), and if I understand Resource correctly it has to be the effect that results from use which does allocate *> use-argument <* release essentially
with sendAndConsume just being use(effect.pure)that makes sense and is potentially dangerous depending on the type of response
the pure http4s dangerous equivalent would be client.run(req).use(IO.pure).flatMap(resp => res.stream.compile.toList) which will result in a empty stream because the underlying resource is already closed If i think this through correctly