You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by Sumit Gupta <su...@hortonworks.com> on 2015/02/18 04:33:18 UTC

[DISCUSS] merging of branch KNOX-481

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

Re: [DISCUSS] merging of branch KNOX-481

Posted by Sumit Gupta <su...@hortonworks.com>.
Thanks for the thorough review Kevin. I took the opportunity to quickly
get the merge out of the way. This does not mean that I am not accepting
further feedback and I will address the feedback you already provided now
in master.

For the record, a long lived branch can be painful to maintain, but for a
feature that can destabilize master I still think it is the way to go
(maybe now that it is merged, it doesn’t seem so bad).

Sumit.


On 2/19/15, 5:40 PM, "Kevin Minder" <ke...@hortonworks.com> wrote:

>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" <la...@gmail.com> wrote:
>
>>Cool - thanks, Sumit!
>>
>>On Wed, Feb 18, 2015 at 9:12 AM, Sumit Gupta
>><su...@hortonworks.com>
>>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" <su...@hortonworks.com>
>>>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" <la...@gmail.com> 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
>>> >><su...@hortonworks.com>
>>> >>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
>>> >>>
>>> >
>>>
>>>
>


Re: [DISCUSS] merging of branch KNOX-481

Posted by Kevin Minder <ke...@hortonworks.com>.
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" <la...@gmail.com> wrote:

>Cool - thanks, Sumit!
>
>On Wed, Feb 18, 2015 at 9:12 AM, Sumit Gupta <su...@hortonworks.com>
>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" <su...@hortonworks.com> 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" <la...@gmail.com> 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
>> >><su...@hortonworks.com>
>> >>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
>> >>>
>> >
>>
>>


Re: [DISCUSS] merging of branch KNOX-481

Posted by larry mccay <la...@gmail.com>.
Cool - thanks, Sumit!

On Wed, Feb 18, 2015 at 9:12 AM, Sumit Gupta <su...@hortonworks.com>
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" <su...@hortonworks.com> 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" <la...@gmail.com> 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
> >><su...@hortonworks.com>
> >>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
> >>>
> >
>
>

Re: [DISCUSS] merging of branch KNOX-481

Posted by Sumit Gupta <su...@hortonworks.com>.
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" <su...@hortonworks.com> 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" <la...@gmail.com> 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
>><su...@hortonworks.com>
>>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
>>>
>


Re: [DISCUSS] merging of branch KNOX-481

Posted by Sumit Gupta <su...@hortonworks.com>.
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" <la...@gmail.com> 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
><su...@hortonworks.com>
>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
>>


Re: [DISCUSS] merging of branch KNOX-481

Posted by larry mccay <la...@gmail.com>.
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 <su...@hortonworks.com>
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
>