Update for OkHttp3 #435
Update for OkHttp3 #435
Conversation
|
By analyzing the blame information on this pull request, we identified @grantland, @wangmengyan95 and @Allsimon to be potential reviewers. |
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! |
|
@rogerhu updated the pull request. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@rogerhu updated the pull request. |
| okHttpClient.setSslSocketFactory(SSLCertificateSocketFactory.getDefault( | ||
| socketOperationTimeout, sslSessionCache)); | ||
| // Broken until OkHttp3 can expose trust managers -- https://github.com/square/okhttp/commit/85f74e2004eaf0d4ff339e8644df3d8e716361e5 | ||
| // builder.sslSocketFactory(SSLCertificateSocketFactory.getDefault(socketOperationTimeout, sslSessionCache)); |
There was a problem hiding this comment.
is this still necessary given the connect timeout is already setup above?
There was a problem hiding this comment.
I'm not entirely sure, but I think this was a relic from our original use of AndroidHttpClient.newInstance(userAgent, sessionCache).
If you are sure that OkHttp client doesn't need a SSL cache, I'd be happy to accept this!
There was a problem hiding this comment.
It doesn't need one except helps obviously with saving SSL handshakes when reconnecting.
In order to use it, I think we have to wait for OkHttp v3.3.0 to drop. See conversation here:
There was a problem hiding this comment.
I don't have enough experience to know whether SSL session caching functionality is a blocker... anyone on the Parse team could provide some insight?
There was a problem hiding this comment.
I think it would be safe to use the default, so we can just remove this bit.
|
@rogerhu updated the pull request. |
|
@rogerhu updated the pull request. |
|
I don't see a change in the ParseHttpClient class to update the OKHTTPCLIENT_PATH field for OkHttp3. I think this means the classpath lookup to make sure OkHttp3 in available would fail. |
|
Thanks for checking. Updated. |
|
@rogerhu updated the pull request. |
|
Mind removing that commented out bit and squashing? After that it LGTM! |
|
Remove commented section |
|
@rogerhu updated the pull request. |
|
Merging. Thanks for the contribution! |
|
Thanks! Any chance to cut a release? |
|
Currently waiting on one PR, but will make a cut next week if it isn't in by then. |
Fixes for OkHttp3