Thanks for the feedback Kevin. Some answers are inline.
On 9/2/15, 10:24 AM, "Kevin Minder" <[email protected]> wrote:
>Sumit,
>
>This looks very promising and is definitely going in the direction I was
>hoping for. A few things I want to confirm.
>
>1) The removal of the need to detect the presence of delegation tokens in
>DefaultDispatch is the result of being able to ³intercept² the SPNego
>challenge because you have setup the httpclient via the factory. This is
>a change from before where the actual ³business² call was actually never
>setup for the SPNego challenge and the SPNego exchange was done out of
>band. We are now doing this ³in band². Do I have that right? If so
>this is much better.
[SG] That is my understanding as well.
>
>2) I still see in DefaultDispatch the use of CappedBufferHttpEntity lines
>200-212. Is this still required and if so how does that end up getting
>used? Being able to remove that is certainly a goal.
[SG] Definitely. As per my comments on the patch, it is one of the TODOs
before this task is done. I had this code out at first and hit some
strange errors during testing and then I minimized my change set for
testing purposes (only to find out that the issue was the environment I
was testing in).
>
>To provide feedback to your questions.
>Q) Should HttpClientFactory.createHttpClient be passed a specific config
>config?
>A) I don¹t have strong feelings here. As you say FilterConfig is likely
>to have what you need now and what you might later. Won¹t anything else
>just be some other flavor of Map anyway?
>
>Q) Do we need http-client-factory configurability at the global level?
>A) No I don¹t think so but on the other hand I can imagine that we might
>end up needing some global configuration like connection and read/write
>timeouts. Doing that via a http-client-factory configuration setting
>seems wrong as it makes it too hard to manage.
[SG] That is a good point. I did mean to take a look at the http client
params that can be passed through from a topology. Another thing I
whittled down due to testing failures was the use of the system
properties. The client builder code would look something like this:
HttpClients.custom().useSystemProperties()
.setDefaultAuthSchemeRegistry(authSchemeRegistry)
.setDefaultCookieStore(new HadoopAuthCookieStore())
.setDefaultCredentialsProvider(credentialsProvider);
Š.etc
Where the key call is ŒuseSystemProperties()¹.
That still does not provide enough control as I believe it only provides
these:
* ssl.TrustManagerFactory.algorithm
* javax.net.ssl.trustStoreType
* javax.net.ssl.trustStore
* javax.net.ssl.trustStoreProvider
* javax.net.ssl.trustStorePassword
* java.home
* ssl.KeyManagerFactory.algorithm
* javax.net.ssl.keyStoreType
* javax.net.ssl.keyStore
* javax.net.ssl.keyStoreProvider
* javax.net.ssl.keyStorePassword
* http.proxyHost
* http.proxyPort
* http.nonProxyHosts
* http.keepAlive
* http.maxConnections
So I can also look into the various http client params available and have
them be something that can be set at the topology level (?).
>
>Kevin.
>
>
>
>On 9/2/15, 7:34 AM, "larry mccay" <[email protected]> wrote:
>
>>This strategy sounds great to me.
>>Abstracting the config object makes sense as well - even if just making
>>it
>>a factoryContext type of thing would be better.
>>
>>I'm not sure that we need gateway level configurability though a default
>>in
>>the service definition would be good.
>>Perhaps, the ability to override it in the topology would help testing
>>but
>>I would let that decision be made based on need.
>>Considering that we haven't needed it yet, maybe we won't.
>>
>>Over course, you do have a MockHttpClientFactory in the patch being used
>>but I see that it is used at the service definition level.
>>
>>On Tue, Sep 1, 2015 at 1:55 PM, Sumit Gupta <[email protected]>
>>wrote:
>>
>>> All,
>>>
>>> I am seeking input/feedback for the work that is going into KNOX-593.
>>> There is an initial patch uploaded and as can probably be gleaned
>>>fairly
>>> quickly, the idea is to move code out from Dispatch implementations and
>>> leverage apache common's HttpClient functionality as much as possible.
>>> Besides the code maintenance win, this hopefully reduces the need to
>>>create
>>> custom dispatches if we allow for HttpClient's to be pluggable.
>>>
>>> Therefore while undertaking this effort, there is an attempt to make
>>>the
>>> HttpClient creation pluggable as well. Hence the new interface for
>>> HttpClientFactory. Right now the create method on factory interface is
>>> being passed the entire FilterConfig object. This felt odd to me from
>>>an
>>> interface cleanliness standpoint but it was my initial stab at the
>>> interface simply because it contained the information that the factory
>>> would likely need. I think I would like to make a more appropriate
>>>config
>>> object for this.
>>>
>>> The only pluggability mechanism in the patch is that of adding the http
>>> client factory class name to the dispatch config in the service
>>>definition.
>>> The question becomes, do we need an overall setting in gateway-site?
>>>Do we
>>> need overrides at the topology level?
>>>
>>> For example, the dispatch config in a service definition could look
>>>like
>>> this:
>>>
>>> <dispatch
>>> http-client-factory="org.apache.hadoop.gateway.MockHttpClientFactory"/>
>>>
>>> Thanks in advance for the feedback.
>>> Sumit.
>>>
