Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[android] Allow a custom configured OkHttpClient to be injected… #3849

Closed

Conversation

jonbe242
Copy link
Contributor

@jonbe242 jonbe242 commented Feb 8, 2016

… into the HTTPContext.

This should resolve issue #3597 for Android according my option no 1.

@jfirebaugh
Copy link
Contributor

Given that we've switched back and forth between OkHttp and other implementations a couple times, I'm not sure that we want to commit to exposing an OkHttp dependency in the public API. @tobrun what do you think?

@jonbe242
Copy link
Contributor Author

jonbe242 commented Feb 9, 2016

I can sympathize with you not wanting to expose OkHttp like that. But then I'd like you to consider my option no 2 in #3597.

An easy way for any SDK user to monitor and adjust HTTP requests is a must have in my opinion. Going back to a native solution would make that much harder, so I don't want that.

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Feb 9, 2016
@tobrun
Copy link
Member

tobrun commented Feb 9, 2016

@jonbe242 first of all, thank you for reaching out and contributing to this repo.

As @jfirebaugh noted, this would indicate that we are committing to always expose a configure the http client library-API in future releases. Moving back to a native would make this very hard since we have to configure the client through JNI. I understand the need for this when you combine that with the requirement of having an own tile server, but I'm not sure how this translates to our own requirements and goals.

Before going further with this, I'm going to ping iOS developers in the original issue #3597.
Also pinging @bleege and @zugaldia for 👀 and roadmap related remarks.

@zugaldia
Copy link
Member

zugaldia commented Feb 9, 2016

@jonbe242 @jfirebaugh @tobrun Though I don't see ourselves going away from OkHttp any time soon (we also opted for it in our directions and geocoder libraries) I agree we don't necessarily want to expose the OkHttp client directly.

If the use case is to be able to attach a custom interceptor (that will allow setting headers, or log requests) I'd prefer to expose a generic API to add and remove interceptors (the interface is here).

@jfirebaugh
Copy link
Contributor

Ok, per the discussion here, closing in favor of #3597, where further discussion can happen about exposing an interceptor API that's not tied to a particular implementation.

@jfirebaugh jfirebaugh closed this Feb 11, 2016
@jonbe242 jonbe242 deleted the 3597-custom-android-http-client branch February 12, 2016 06:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants