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

A PoC of Gears integration #2142

Closed
wants to merge 1 commit into from
Closed

A PoC of Gears integration #2142

wants to merge 1 commit into from

Conversation

adamw
Copy link
Member

@adamw adamw commented Apr 20, 2024

See https://github.com/lampepfl/gears

Currently sttp-client defines a couple of backend types: SyncBackend, Backend[F[_]], WebSocketSyncBackend, StreamBackend[F[_], +S], WebSocketStreamBackend[F[_], S].

They all inherit from a GenericBackend[F[_], +P] (where P are supported sttp-capabilities, such as streaming and websockets), but sttp has specialisied "concrete" types for better auto-complete and general developer experience.

My first first thought was to add a CapabilityBackend[C] to the hierarchy, but this would mean also adding a WebSocket/Streaming variants, and adding the new variants for all generic backend wrappers. This would additionally complicate the already complicated setup.

So instead, I went with representing the "Gears effect" as a type constructor (I called it Deferred), and using Backend[F[_]]:

One thing that is not ideal is the .send variant that I had to add, which would work for any capability:

def send[C](backend: Backend[[X] =>> C ?=> X])(using C): Response[T] = backend.send(this)

This is needed so that both of these compile:

val result: Response[Either[String, String]] = basicRequest.get(uri"https://x.org/").body("123").send(b)
val result = basicRequest.get(uri"https://x.org/").body("123").send(b)

(without it only the second, not-explicitly-typed one compiles)

I also tried adding a gears-specific extension method (which would have a using Async, instead of the generic formulation above), but unfortunately the .send methods defined directly in the Request class have priority, so this method was never taken into consideration.

Anyway, summing up, to add gears integration into sttp-client with the approach above, we have to:

  • add a capability-generic .send variant
  • in the gears integration, create a type alias which allows representing a capability-aware computation as an effect
  • define the backend, either wrapping a synchronous backend, or implementing custom logic using the capability

@adamw adamw closed this Jan 10, 2025
@adamw
Copy link
Member Author

adamw commented Jan 10, 2025

@odersky @natsukagami Before releasing sttp-client4, it would be great to determine if there's something in the library itself that needs to be changed to provide support for a future Gears release (it will be much harder once the release is done).

I think one of the basic questions is when we have a hypothetical gearsBackend, which can be used to send requests, what should be the return type of .send? That is, we can have:

val response: Response[...] = myRequest.send(gearsBackend) // Async must be in scope, `send` has a Async ?=> type

Where the request is sent at the point of invocation. Or, we can have the lazy version:

val response: Async ?=> Response[...] = myRequest.send(gearsBackend)

where we defer effects. I'm quite sure that the first version is what we want, but better to double-check :)

The second question is how a gears backend would look like. In the PoC I have it captured simply as a Backend[[X] =>> Async ?=> X], which on one hand works, but on the other isn't the most user-friendly signature. But then, to properly support all capability variants, we also have to take into account websockets & streaming, which is tricky.

Well, if you have any thoughts, let me know :)

@adamw adamw reopened this Jan 10, 2025
@natsukagami
Copy link
Contributor

Hi! I definitely prefer the first variant, and I suppose the second variant is also pretty hard to use? (as in it's not that easy to keep the laziness)

As for the backend, my (also really hacky) PoCs all used the Backend[[X] =>> Async ?=> X] and I think it's fine. I will eventually add a type alias to Gears for [X] =>> Async ?=> X, but changing that is hopefully backwards-compatible.

@adamw
Copy link
Member Author

adamw commented Jan 10, 2025

@natsukagami Ah, great to hear you did some tests as well :)

So if we just have a backend of type Backend[[X] =>> Async ?=> X], then request.send(backend) will have the type Async ?=> Response[String]. So the Async implicit will be automatically applied at the call-site, unless the value is explicitly typed with Async ?=> .... So maybe that's enough, and no code changes in sttp are needed?

@natsukagami
Copy link
Contributor

I think that's what you want most of the time. Either request.send(backend) which acts like a normal using Async function, or wrap it in a Future like Future(request.send(backend)).
I'm pretty happy with it :)

@adamw
Copy link
Member Author

adamw commented Jan 10, 2025

I guess the question is, if you have a request.send(backend) invocation, will IDE auto-completion work correctly, and if you ask the IDE to infer the type, will it give the "expected" one, that is Response[String], not Async ?=> Response[String]

On the other hand, having both:

def send[F[_]](backend: Backend[F]): F[Response[T]]
def send[C](backend: Backend[[X] =>> C ?=> X])(using C): Response[T]

methods on the Request[T] type might cause confusion in the compiler? will it always choose the more specific variant, as is the capability-variant even more specific? (not to mention that it will be hard to implement in sttp in a Scala2 & Scala3-compatible way)

@natsukagami
Copy link
Contributor

if you ask the IDE to infer the type, will it give the "expected" one, that is Response[String], not Async ?=> Response[String]

This does iirc

Having both ... methods

will give you a compiler error with "ambiguous overload" at call site I think.

@adamw
Copy link
Member Author

adamw commented Jan 10, 2025

Yeah just checked, works fine. I guess I'll close this then, always nice to have one less item on the todo list ;)

@adamw adamw closed this Jan 10, 2025
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.

2 participants