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. 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. 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. 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. >>
