cpp_client_telemetry icon indicating copy to clipboard operation
cpp_client_telemetry copied to clipboard

Draft - Android - allow supply a different request factory to HttpClient

Open anod opened this issue 2 years ago • 5 comments

Task #1049 Enable HTTP/2.0 support for the SDK.

  • Add option to provide a different request factory to HttpClient
    • Add HttpClientRequest and HttpClientRequest.Factory interfaces
    • Supply a default AndroidUrlConnection factory
  • Add a new maesdk-okhttp module with okhttp implementation to avoid pulling/managing external dependencies

image

TODO: [ ] Test okhttp: parsing request, cancellation, http/2 connection are being reuse [ ] Add unit test for okhttp [ ] Publish okhttp module

anod avatar Sep 13 '22 06:09 anod

Why don't we have okhttp implementation be the default implementation? @lalitb ?

sid-dahiya avatar Sep 13 '22 15:09 sid-dahiya

Why don't we have okhttp implementation be the default implementation? @lalitb ?

need to cosider version management: default will require all clients to update to 4.10, and vise versa in case of new version of okhttp, the library needs to be updated...

anod avatar Sep 13 '22 15:09 anod

Why don't we have okhttp implementation be the default implementation? @lalitb ?

need to cosider version management: default will require all clients to update to 4.10, and vise versa in case of new version of okhttp, the library needs to be updated...

Got it, in that case, maybe this can be the default in the next major SDK version.

sid-dahiya avatar Sep 13 '22 15:09 sid-dahiya

@sid-dahiya Do you know if someone from office telemetry team with Android expertise (Saurabh?) can help review this code?

lalitb avatar Sep 14 '22 17:09 lalitb

Why don't we have okhttp implementation be the default implementation? @lalitb ?

need to cosider version management: default will require all clients to update to 4.10, and vise versa in case of new version of okhttp, the library needs to be updated...

Got it, in that case, maybe this can be the default in the next major SDK version.

Yes, this sounds like good plan to give users option to migrate separately and test the implementation, and then make it default

anod avatar Sep 15 '22 09:09 anod