Initial implementation of an OkHttp-backed curl clone. #514
Conversation
| Request request = getConfiguredRequest(); | ||
| client.enqueue(request, this); | ||
|
|
||
| // Immediately begin triggering an executor shutdown so that after execution of the above |
There was a problem hiding this comment.
hmm. . . policy here is tricky. I don't think they should be daemon threads unless they're idle. Unfortunately that isn't an option. Dumb.
|
Nice! |
|
LGTM |
|
I bet @brianm will love this |
|
|
||
| private static String protocols() { | ||
| return Joiner.on(", ") | ||
| .join(FluentIterable.from(Arrays.asList(Protocol.values())) |
There was a problem hiding this comment.
nit: Lists.transform would be a little shorter
|
heh.. the following does actually work to add NPN support! + <configuration>
+ <flags>-Xbootclasspath/p:$0</flags>
+ </configuration> |
|
ps right now, there's no way to shutdown the spdy daemon threads, so we should System.exit after main or create some thing to shut the threads down. |
|
addressed my comments. Note that the async nature of this is not shutting down the reader thread. This causes okcurl to hang on spdy connections. It would be simpler to just make this use UrlConnection, so that it can call disconnect and close properly. Unless someone is opposed, I'd prefer making this change. |
|
@JakeWharton judgement call time. I added a commit which uses HttpUrlConnection and doesn't need the changes to Dispatcher. Lemme know how you feel about it. |
|
@adriancole no not HttpURLConnection! Our new async API is the future. Maybe we want a method on ConnectionPool to prune idle connections? |
|
going to try something that hopefully can kill the last commit! |
| if (allowInsecure) { | ||
| client.setSslSocketFactory(createInsecureSslSocketFactory()); | ||
| } | ||
| client.setConnectionPool(ConnectionPool.getDefault()); |
There was a problem hiding this comment.
if we don't set this reference, there's no way to clean shutdown persistent connections
There was a problem hiding this comment.
Add as comment?
|
OK. looks like we are in business! ~/Development/okhttp/okcurl jw/okcurl target/okcurl-2.0.0-SNAPSHOT-jar-with-dependencies.jar -i https://google.com
HTTP/1.1 301 Moved Permanently
OkHttp-Selected-Transport: spdy/3.1
OkHttp-Selected-Protocol: spdy/3.1
alternate-protocol: 443:quic
cache-control: public, max-age=2592000
content-length: 220
content-type: text/html; charset=UTF-8
date: Sat, 08 Feb 2014 22:35:00 GMT
expires: Mon, 10 Mar 2014 22:35:00 GMT
location: https://www.google.com/
server: gws
x-frame-options: SAMEORIGIN
x-xss-protection: 1; mode=block
OkHttp-Sent-Millis: 1391898900226
OkHttp-Received-Millis: 1391898900882
OkHttp-Response-Source: NETWORK 301
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="https://www.google.com/">here</A>.
</BODY></HTML> |
| } | ||
|
|
||
| private void close() { | ||
| client.getConnectionPool().evictAll(); // close any persistent connections. |
There was a problem hiding this comment.
nit: capitalize sentence
|
lgtm! |
|
okie notes addressed. |
|
awesome, jake! now we have a curl that supports spdy and http/2! |
|
Got a single test failing. |
|
it's a flake.. probably need to throw another round of flake-be-gone soon. |
Initial implementation of an OkHttp-backed curl clone.
Supports (and mirrors directly) a few of the options provided by curl. Most notably missing is the ability to change the HTTP method, force the protocol, supply a body, and verbose logging of connection information. These will be added in subsequent changes.