Reuse HTTP client for download

[asmello]

The implementation of the download method for the Artifactory registry creates a new HTTP client in each call. To take advantage of connection pooling, it should reuse the client instead.

buffrs/src/registry/artifactory.rs

Line 91 in ef3e64f

let response = reqwest::Client::new()

[CouldBeFree]

What if use something like a singleton? I.e. I can implement a struct that will have a one-field response. The initial state of this field will be null (None), after the first call of download it will establish a connection and set response to the value of reqwest::Client::new(), after the second call of download function I can use the response value.
What do you think?

[asmello]

What if use something like a singleton? I.e. I can implement a struct that will have a one-field response. The initial state of this field will be null (None), after the first call of download it will establish a connection and set response to the value of reqwest::Client::new(), after the second call of download function I can use the response value. What do you think?

To be clearer, this issue is concerned with reusing the client to avoid establishing a brand new connection with each request. I expect in practice responses tend not to repeat since each download will hit a different URL for a different dependency.

[CouldBeFree]

What if use something like a singleton? I.e. I can implement a struct that will have a one-field response. The initial state of this field will be null (None), after the first call of download it will establish a connection and set response to the value of reqwest::Client::new(), after the second call of download function I can use the response value. What do you think?

To be clearer, this issue is concerned with reusing the client to avoid establishing a brand new connection with each request. I expect in practice responses tend not to repeat since each download will hit a different URL for a different dependency.

Thank you for clarification. I am new to Rust, it's look like this issue is to difficult for me at this point

[koiuo]

In the main branch I see that the client is already a part of the Artifactory struct.

Is this still relevant?

[asmello]

Partially, though less so. A new client is still created for each dependency explored when constructing the dependency graph as part of the install() command.

I had a PR that solved this as part of moving the responsibility of instancing clients to a central RegistryProvider that would lazily create a single client as necessary for each registry URI, but I abandoned that PR because it assumed we'd want co-existing Registry implementations, which was not true. That said, there is nothing intrinsically wrong with that design, it's just that instead of trait objects it would be better off using generics. Only reason I haven't pushed that change forward is because this is not a pressing optimisation at the moment. So if you feel like contributing it, go ahead, I'd be happy to review.