Sumit, I went through it fairly carefully and I didn¹t find anything which concerned me terribly. I like the direction and the implementation. I left a bunch of line item comments in https://issues.apache.org/jira/browse/KNOX-481, but I know you have been suffering tracking master. So I¹m +1 on a merge with a followup change to address my ³cleanup² level feedback.
+1 for merge. Kevin. On 2/18/15, 9:28 AM, "larry mccay" <[email protected]> wrote: >Cool - thanks, Sumit! > >On Wed, Feb 18, 2015 at 9:12 AM, Sumit Gupta <[email protected]> >wrote: > >> Sorry it seems that the server stripped my attachment, possibly because >>I >> named it poorly. >> >> Here it is (hopefully). Also if you would like to create a patch locally >> to use, you can checkout the branch KNOX-481 and then use: >> >> git format-patch master --stdout > KNOX-nnn.patch >> >> >> >> On 2/17/15, 11:57 PM, "Sumit Gupta" <[email protected]> wrote: >> >> >So I tried attaching the big and ugly patch (due to all the commits and >> >merges that I failed to squash) and got rejected by the mail server >>due to >> >the size of the attachment. >> > >> >I have however attached a plain 'git diff' from master which is >> >hopefully a lot easier to read. Let me know if something else will >>make it >> >easier. The branch itself of course is pushed and may just be easy >>enough >> >to look at in your IDE. >> > >> >Sumit. >> > >> > >> > >> >On 2/17/15, 10:55 PM, "larry mccay" <[email protected]> wrote: >> > >> >>This sounds like an excellent step forward for the deployment >>machinery >> >>of >> >>Knox - good work! >> >>It will also provide even greater compatibility with Hadoop platform >> >>deployments. >> >> >> >>We should continue this work to define collections of versioned >>services >> >>as >> >>a single Hadoop platform definition - composites. >> >>I would also like to see providers referenced as reusable >>configurations >> >>of >> >>security policy as we have discussed offline. >> >> >> >>I will spend some time reviewing this work tomorrow. >> >>Is there a single patch available for the review? >> >> >> >>On Tue, Feb 17, 2015 at 10:33 PM, Sumit Gupta >> >><[email protected]> >> >>wrote: >> >> >> >>> I'm looking for comments, feedback and essentially a review of the >> >>>branch >> >>> KNOX-481. It contains work related to the jira of the same name. >> >>> >> >>> At the highest level, the changes involve removing boiler plate code >> >>>for >> >>> adding a service that involved deployment and dispatch contributors >>and >> >>> replacing it by writing an xml file. The file needs to be called >> >>> 'service.xml' (in this initial version) and be placed alongside the >> >>> corresponding rewrite.xml file if there is one. >> >>> >> >>> The existing service 'definitions' have been converted to use this >> >>>format >> >>> and can be found in a new maven module called >> >>>gateway-service-definitions. >> >>> Just to recap, these services are: >> >>> >> >>> 1. Hbase >> >>> 2. Hive >> >>> 3. Oozie >> >>> 4. Webhcat >> >>> 5. Webhdfs >> >>> 6. Yarn >> >>> >> >>> The structure of the files (and the files themselves) also make >>their >> >>>way >> >>> into in the knox assembly under the 'data' directory. >> >>> >> >>> Without delving into the xml file too much, I am hoping that the >> >>>existing >> >>> service files provide enough of an example as does the fake service >>for >> >>>the >> >>> test case. >> >>> >> >>> Other than that, some minor things of note are: >> >>> >> >>> 1. The httpclient is now shared a bit more than it was previously >> >>>(it >> >>> was created for each request before) >> >>> 2. The service definitions can now be versioned and the topology >>can >> >>> specify a version. If none is specified, the latest should be >>picked. >> >>> 3. The dispatch class can still be customized either by >>specifying a >> >>> contributor or a class name. >> >>> 4. To make the writing of a custom dispatch easier, there is a >> >>> configuration injection framework (thanks to Kevin Minder) that will >> >>>inject >> >>> objects onto the dispatch instance from FilterConfig or >>ServletContext. >> >>> >> >>> Sumit >> >>> >> > >> >>
