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

Re-introduce support for sync clients #112

Closed
garyanaplan opened this issue Jan 23, 2020 · 4 comments
Closed

Re-introduce support for sync clients #112

garyanaplan opened this issue Jan 23, 2020 · 4 comments
Labels
client http issues with the client question Direction unclear; possibly a bug, possibly could be improved.

Comments

@garyanaplan
Copy link

garyanaplan commented Jan 23, 2020

I started using this crate back in the 0.17 release (pre-async) and it is used in a heavily multi-threaded and UNIX signal-aware environment. For our use case, the synchronous crate works perfectly, but I'm aware that we are falling some way behind HEAD and worried that we are going to get left behind as the crate continues to evolve.

I've looked at introducing async support to our code and it's not trivial.

reqwest supports a "blocking" Client, so I hope it would be possible to use this to re-introduce sync support to kube-rs. Would you be interested in re-introducing sync support as a first class citizen?

I'm happy to work on providing/supporting an implementation but I would probably need some guidance/mentoring on feasibility/approach.

@garyanaplan garyanaplan changed the title is tokio a requirement for working with this crate? Re-introduce support for sync clients Jan 23, 2020
@clux
Copy link
Member

clux commented Jan 23, 2020

Does it have to be fully sync? Tokio itself is not required, but at least a lightweight runtime to schedule tasks is required. The api evolved quite a bit - in a good direction - after async, and must admit that i'm not particularly keen to supporta a reqwuest sync world, particularly without having properly thought about #102, #66, and #100. All of these issues might impact reqwest as the underlying http implementation (we are considering abstracting it away).

If after that, there's an easy approach to maintain compatibility, then I'm all for at least trying it. But I don't want to commit to going down that route right now 😿

@garyanaplan
Copy link
Author

I think tokio is required at the moment because of the use of async reqwest. I tried figuring out a way to connect to my cluster which didn't use tokio (using block_on from the futures crate) and I got error messages (originating in reqwest) which complained that the runtime wasn't tokio. I used a tokio runtime to get me past the auth part of my code and then found I had the same problem when I started interacting with kube-rs, presumably because it is using async reqwest under the covers here.

I wonder if this is a problem that would disappear if the underlying HTTP implementation didn't explicitly rely on tokio? As an approach that could work for me.

@clux
Copy link
Member

clux commented Jan 23, 2020

Yeah, it's not ideal that we hide away the client like this. I like the idea of just defining the protocol as much as possible and letting users choose the client. Hoping #100 is going to be doable. Tower has some tokio deps AFAIKT, but only some of the leaf crates like timer.

If that works then that should hopefully remove the dependency on tokio.

@clux clux added the client http issues with the client label Feb 26, 2020
@clux clux mentioned this issue Feb 8, 2021
@clux clux added the question Direction unclear; possibly a bug, possibly could be improved. label Feb 8, 2021
@clux
Copy link
Member

clux commented Feb 8, 2021

As an update, we did get rid of reqwest, which brings us a bit closer towards a sans-io possibility.

I did have a look at how reqwest was exposing its blocking client; it's using tokio and blocking on every await point, so it's possible a runtime selection and and manual thread spawning around it can help you, but it would still encapsulate a bunch of futures + async under the hood, so not sure if that's even helpful.

At any rate, I'm going to close this as there are no planned activity towards a sync, and it's possibly more of an application concern about what runtimes people want underneath, but a runtime requirement isn't going away.

As a follow-up, the work in #406 regarding a sans-io Client, might yield developments towards runtime agnosticity, but with the caveat that this is not really the main goal for us atm.

@clux clux closed this as completed Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client http issues with the client question Direction unclear; possibly a bug, possibly could be improved.
Projects
None yet
Development

No branches or pull requests

2 participants