Kamel-Media / Kamel

Kotlin asynchronous media loading and caching library for Compose.
Apache License 2.0
595 stars 23 forks source link

Allow construction of HttpFetcher with already made client #16

Closed Syer10 closed 1 year ago

Syer10 commented 2 years ago

For my application I have a Ktor HttpClient that I use everywhere since it is configured for a specific server. Previously I used for construction of the HttpFetcher.

httpFetcher(httpClient.engine) {
   install(httpClient)
}

But recently a feature I needed was added to the Kamel HttpClient and it fails to use that feature. Sadly I cant construct the HttpFetcher myself since its internal. To get past this I copied the HttpFetcher and am using it directly passing my HttpClient to it. It would be nice to add this to Kamel like so

httpFetcher(httpClient)
alialbaali commented 1 year ago

Kamel uses Ktor's HttpClient. You can configure the HttpClient by a lambda, if that's what you're asking.

Syer10 commented 1 year ago

I already have a constructed Ktor HttpClient, I would like to use that one in the HttpFetcher. The current API doesn't allow that

alialbaali commented 1 year ago

Ok, I understand now. I'll add a function that takes an HttpClient as a parameter.

alialbaali commented 1 year ago

Do you think I should also provide functions for all HttpClient constructor functions?

Syer10 commented 1 year ago

The functions look cleaner, would there be any reason why you shouldn't?

alialbaali commented 1 year ago

Well, one reason is that I provided the function you asked for, which already can be used to construct the HttpClient using its own custom parameters. So, I don't think there's a need for that. What do you think?

Syer10 commented 1 year ago

Maybe just a default one and one that you can pass a already constructed client then. The others are not as useful

alialbaali commented 1 year ago

Version 0.4.0 is out. Please check it and let me know!

xxfast commented 1 year ago

Working as intended 👍 Nice work @alialbaali