Adding HttpExecutor to be used for interchangeable implementations #285
Conversation
ygy
force-pushed
the
feature/http-executor
branch
from
45f186d
to
567e39c
Compare
|
|
||
| void setParams(HttpParams httpParams); | ||
|
|
||
| void setRedirectStrategy(LaxRedirectStrategy laxRedirectStrategy); |
There was a problem hiding this comment.
I'm not convinced we want to tie this interface to the org.apache.http classes. I was assuming we'd make it more general. And that we would not extend HttpClient, which would presumably make it a lot harder to supply implementations which do not use org.apache.http.
ygy
force-pushed
the
feature/http-executor
branch
3 times, most recently
from
508bab9
to
9f7a079
Compare
ygy
force-pushed
the
feature/http-executor
branch
from
230b6f0
to
7c123ad
Compare
ygy
changed the title
| import com.google.common.base.Optional; | ||
| import com.google.common.collect.Iterables; | ||
|
|
||
| public class HttpExecutorFactory { |
|
Looks like a great start. |
ygy
force-pushed
the
feature/http-executor
branch
from
6bde585
to
e50161b
Compare
| .build()); | ||
| return createHttpToolRespose(response); | ||
| }}; | ||
| getPoller().scheduleAtFixedRate(pollJob, new DelegatingPollHandler(handlers), minPeriod); |
|
A few fixes in here ygy#3 |
| return (Poller<HttpToolResponse>) super.getPoller(); | ||
| } | ||
|
|
||
| private HttpToolResponse createHttpToolRespose(HttpResponse response) throws IOException { |
There was a problem hiding this comment.
very minor - syntax error - "response"
| instructions. */ | ||
| if (responseCode == 422) { | ||
| throw new IOException(" Unprocessable Entity: " + response.reasonPhrase()); | ||
| } |
There was a problem hiding this comment.
Still don't see why we need special handling of 422? (to discuss).
ygy
force-pushed
the
feature/http-executor
branch
from
d7397d8
to
26f0b5e
Compare
| Multimap<String, String> headers(); | ||
|
|
||
| @Nullable | ||
| Credentials.BasicAuth credentials(); |
There was a problem hiding this comment.
Don't like how BasicAuth is hardcoded in the API. Username + password could be used for digest or a number of other schemes. The credentials are (mostly) orthogonal to the scheme.
- Rename
BasicAuthtoUsernamePassword(Credentials). - Change Credentials to interface (with getUser, getPassword method) and implement it in the above.
- Use Credentials in the API.
|
Almost there, only concerned about the content length being available on responses. |
ygy
force-pushed
the
feature/http-executor
branch
2 times, most recently
from
3447ecf
to
4132491
Compare
| ByteStreams.copy(response.getContent(), out); | ||
| content = out.toByteArray(); | ||
| } catch (IOException e) { | ||
| throw Throwables.propagate(e); |
There was a problem hiding this comment.
In Brooklyn the recommended is Exceptions.propagate, but this works as well in this case.
ygy
force-pushed
the
feature/http-executor
branch
from
5cf8c3c
to
07d2207
Compare
ygy
force-pushed
the
feature/http-executor
branch
from
07d2207
to
f9f3b99
Compare
|
Thanks @ygy, merging. |
No description provided.