hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Karl Wright <daddy...@gmail.com>
Subject Re: Expect-continue doesn't seem operative using 4.3.x builder structures
Date Thu, 22 May 2014 22:16:27 GMT
The resolution to this problem was simple -- after I audited both
HttpClient and SolrJ code.

(0) Expect/continue handling, in the POST-case I was looking at, was in
fact *off*.  It was disguised by the fact that the thread ID differed and I
did not note that in the log when I looked at it.
(1) In InternalHttpClient, if *any* configuration parameters are set, then
configuration parameters are used, and the RequestConfig is *thrown away*.
(2) In HttpSolrServer, randomly, it sets *one* configuration parameter,
namely one turning on redirection handling.  This was actually in the code
I included in the ticket, and it was sufficient to completely throw away my
builder-created configuration.
(3) The solution was to create a modified version of HttpSolrServer and fix
the code so that it didn't do that.

Any of the following would have been very helpful:

(a) At least, output a log message when both parameters AND Config object
were present.
(b) Documentation (javadoc) making the implemented behavior clear.
(c) Ideally, if you set a parameter and you've already got a Config object,
an IllegalStateException should be thrown.

This would have likely made it much quicker to diagnose than repeated
debugging sessions, via patched diagnostic jars, with clients half a world
away.  Better to fail immediately than give the impression that things were
okay when they were not.  Thoughts?

Karl





On Thu, May 22, 2014 at 9:05 AM, Karl Wright <daddywri@gmail.com> wrote:

> Hi Oleg,
>
> This is the order of events in the code in question -- which has not
> changed, by the way, since we went for 4.2.x to 4.3.3:
>
> - ManifoldCF creates an HttpClient instance
> - ManifoldCF hands the HttpClient instance to SolrJ (a solr library)
> - SolrJ posts to Solr using multipart post
>
> We don't get to specify how SolrJ wraps its entities, or even IF it wraps
> its entities.  (My recollection is that it doesn't do any kind of explicit
> wrapping.)  And in any case I certainly don't understand the reasons behind
> every line of SolrJ client code.
>
> If your claim is that you can do multipart entity posts in HttpClient
> without using a class derived from HttpRequestWrapper, fine, then I will
> look into the SolrJ code and find a way to work around SolrJ's
> implementation.
>
> Please leave HTTPCLIENT-1510 open until I am done with my SolrJ review.
>
> Karl
>
>
>
> On Thu, May 22, 2014 at 8:56 AM, Oleg Kalnichevski <olegk@apache.org>wrote:
>
>> On Thu, 2014-05-22 at 08:43 -0400, Karl Wright wrote:
>> > Hi Oleg,
>> >
>> > We *are* using POST - multipart post.  And this apparently extends
>> > HttpRequestWrapper.  Which is why expect/continue is not working for us.
>> >
>>
>> Why? What for?  Even if you are absolutely certain wrapping is necessary
>> in this case why then not use HttpRequestWrapper#wrap instead of class
>> extension? The constructor of this class even intentionally made private
>> to discourage people from its extension.
>>
>> > If you have a better solution, please let me know what it is.
>> >
>>
>> What shall I do about HTTPCLIENT-1510? API elegancy, which is subjective
>> and contentious topic, aside it is clearly not a bug with HttpClient.
>>
>> Oleg
>>
>> > Karl
>> >
>> >
>> >
>> > On Thu, May 22, 2014 at 8:40 AM, Oleg Kalnichevski <olegk@apache.org>
>> wrote:
>> >
>> > > On Thu, 2014-05-22 at 08:27 -0400, Karl Wright wrote:
>> > > > Let me clarify.  Right now, you've a wrapper hierarchy that is
>> totally
>> > > > distinct from the original request hierarchy.  You *could* allow
>> > > everything
>> > > > wrapped with HttpRequestWrapper to allow expect/continue, in which
>> case
>> > > you
>> > > > lose the ability to have specificity for different kinds of wrapped
>> > > > requests.  Or (much better) you could have all HttpRequest objects
>> have a
>> > > > "supportExpectContinue" method, which in the wrapper would wind up
>> > > calling
>> > > > the embedded request's supportExpectContinue method.  Seems much
>> better,
>> > > no?
>> > > >
>> > >
>> > > Why is exactly instanceof bad or less flexible? It enables certain
>> > > requests to provide optional behavior such as ability to enclose a
>> > > request entity, which by the current official HTTP spec applies to
>> POST
>> > > and PUT _only_.
>> > >
>> > > So, what is better, round or green?
>> > >
>> > > Oleg
>> > >
>> > > > Karl
>> > > >
>> > > >
>> > > > On Thu, May 22, 2014 at 8:23 AM, Karl Wright <daddywri@gmail.com>
>> wrote:
>> > > >
>> > > > > >>>>>>
>> > > > > Are you sure about that? What would this method do for GET
>> requests
>> > > > > given than those requests are not even supposed to enclose an
>> entity?
>> > > > > <<<<<<
>> > > > >
>> > > > > It would return false for any request implementation that did
not
>> > > support
>> > > > > expect-continue, of course.
>> > > > > The advantage of this kind of structure is that it does not rely
>> on the
>> > > > > implicit instanceof operator, but rather an explicit method
>> > > implementation,
>> > > > > so it is clearer (and more flexible).
>> > > > >
>> > > > > Karl
>> > > > >
>> > > > >
>> > > > >
>> > > > > On Thu, May 22, 2014 at 8:19 AM, Oleg Kalnichevski <
>> olegk@apache.org
>> > > >wrote:
>> > > > >
>> > > > >> On Thu, 2014-05-22 at 07:55 -0400, Karl Wright wrote:
>> > > > >> > FWIW, a better way for this kind of thing to be done
would be
>> for
>> > > the
>> > > > >> > request object to have a method, e.g.
>> "supportsExpectContinue()",
>> > > that
>> > > > >> you
>> > > > >> > would call, instead of relying on class names and hierarchy
...
>> > > > >> >
>> > > > >>
>> > > > >> Are you sure about that? What would this method do for GET
>> requests
>> > > > >> given than those requests are not even supposed to enclose
an
>> entity?
>> > > > >>
>> > > > >> Oleg
>> > > > >>
>> > > > >>
>> > > > >>
>> > > > >>
>> ---------------------------------------------------------------------
>> > > > >> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
>> > > > >> For additional commands, e-mail: dev-help@hc.apache.org
>> > > > >>
>> > > > >>
>> > > > >
>> > >
>> > >
>> > >
>> > > ---------------------------------------------------------------------
>> > > To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
>> > > For additional commands, e-mail: dev-help@hc.apache.org
>> > >
>> > >
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
>> For additional commands, e-mail: dev-help@hc.apache.org
>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message