You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Colm O hEigeartaigh <co...@apache.org> on 2017/02/01 17:47:02 UTC

Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/
-----------------------------------------------------------

(Updated Feb. 1, 2017, 5:47 p.m.)


Review request for ranger.


Changes
-------

Fixing the patch with the latest master


Bugs: RANGER-1318
    https://issues.apache.org/jira/browse/RANGER-1318


Repository: ranger


Description
-------

Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.


Diffs (updated)
-----

  agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
  embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 54c190d 
  security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
  security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 8fd6196 
  ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824 
  unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
  unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 4b56936 

Diff: https://reviews.apache.org/r/55669/diff/


Testing
-------


Thanks,

Colm O hEigeartaigh


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Colm O hEigeartaigh <co...@apache.org>.

> On March 8, 2017, 4:15 a.m., Qiang Zhang wrote:
> > Hi Colm,
> > There are following warnings when I apply the patch.
> > Applying: RANGER-1318 - Consolidate XML configuration parsing
> > .git/rebase-apply/patch:55: trailing whitespace.
> > 	
> > .git/rebase-apply/patch:57: trailing whitespace.
> > 	
> > .git/rebase-apply/patch:69: trailing whitespace.
> > 	
> > .git/rebase-apply/patch:75: trailing whitespace.
> > 			
> > .git/rebase-apply/patch:100: trailing whitespace.
> > 					
> > warning: squelched 4 whitespace errors
> > warning: 9 lines add whitespace errors.
> > 
> > Could you eliminate these warnings before ship it?

OK done, thanks!


- Colm


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/#review168243
-----------------------------------------------------------


On March 8, 2017, 9:55 a.m., Colm O hEigeartaigh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55669/
> -----------------------------------------------------------
> 
> (Updated March 8, 2017, 9:55 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1318
>     https://issues.apache.org/jira/browse/RANGER-1318
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 635e63f 
>   security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
>   security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 29fc78a 
>   ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java d839b39 
>   unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
>   unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 11e0c92 
> 
> 
> Diff: https://reviews.apache.org/r/55669/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Qiang Zhang <zh...@zte.com.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/#review168243
-----------------------------------------------------------



Hi Colm,
There are following warnings when I apply the patch.
Applying: RANGER-1318 - Consolidate XML configuration parsing
.git/rebase-apply/patch:55: trailing whitespace.
	
.git/rebase-apply/patch:57: trailing whitespace.
	
.git/rebase-apply/patch:69: trailing whitespace.
	
.git/rebase-apply/patch:75: trailing whitespace.
			
.git/rebase-apply/patch:100: trailing whitespace.
					
warning: squelched 4 whitespace errors
warning: 9 lines add whitespace errors.

Could you eliminate these warnings before ship it?

- Qiang Zhang


On March 7, 2017, 5:49 p.m., Colm O hEigeartaigh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55669/
> -----------------------------------------------------------
> 
> (Updated March 7, 2017, 5:49 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1318
>     https://issues.apache.org/jira/browse/RANGER-1318
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 635e63f 
>   security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
>   security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 29fc78a 
>   ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java d839b39 
>   unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
>   unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 11e0c92 
> 
> 
> Diff: https://reviews.apache.org/r/55669/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Qiang Zhang <zh...@zte.com.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/#review168279
-----------------------------------------------------------


Ship it!




Ship It!

- Qiang Zhang


On March 8, 2017, 9:55 a.m., Colm O hEigeartaigh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55669/
> -----------------------------------------------------------
> 
> (Updated March 8, 2017, 9:55 a.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1318
>     https://issues.apache.org/jira/browse/RANGER-1318
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 635e63f 
>   security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
>   security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 29fc78a 
>   ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java d839b39 
>   unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
>   unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 11e0c92 
> 
> 
> Diff: https://reviews.apache.org/r/55669/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/
-----------------------------------------------------------

(Updated March 8, 2017, 9:55 a.m.)


Review request for ranger.


Changes
-------

Fixing trailing whitespace errors.


Bugs: RANGER-1318
    https://issues.apache.org/jira/browse/RANGER-1318


Repository: ranger


Description
-------

Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.


Diffs (updated)
-----

  agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
  embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 635e63f 
  security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
  security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 29fc78a 
  ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java d839b39 
  unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
  unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 11e0c92 


Diff: https://reviews.apache.org/r/55669/diff/6/

Changes: https://reviews.apache.org/r/55669/diff/5-6/


Testing
-------


Thanks,

Colm O hEigeartaigh


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/
-----------------------------------------------------------

(Updated March 7, 2017, 5:49 p.m.)


Review request for ranger.


Bugs: RANGER-1318
    https://issues.apache.org/jira/browse/RANGER-1318


Repository: ranger


Description
-------

Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.


Diffs (updated)
-----

  agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
  embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 635e63f 
  security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
  security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 29fc78a 
  ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java d839b39 
  unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
  unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 11e0c92 


Diff: https://reviews.apache.org/r/55669/diff/5/

Changes: https://reviews.apache.org/r/55669/diff/4-5/


Testing
-------


Thanks,

Colm O hEigeartaigh


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/
-----------------------------------------------------------

(Updated Feb. 13, 2017, 1:07 p.m.)


Review request for ranger.


Changes
-------

I updated the patch again to get it to apply to master...please take a look so that I can apply it.


Bugs: RANGER-1318
    https://issues.apache.org/jira/browse/RANGER-1318


Repository: ranger


Description
-------

Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.


Diffs (updated)
-----

  agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
  embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 9668e47 
  security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
  security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 29fc78a 
  ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java a4b12b2 
  unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
  unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 175307f 

Diff: https://reviews.apache.org/r/55669/diff/


Testing
-------


Thanks,

Colm O hEigeartaigh


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/
-----------------------------------------------------------

(Updated Feb. 8, 2017, 9:32 a.m.)


Review request for ranger.


Changes
-------

Updated patch to work with latest master.


Bugs: RANGER-1318
    https://issues.apache.org/jira/browse/RANGER-1318


Repository: ranger


Description
-------

Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.


Diffs (updated)
-----

  agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
  embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 9668e47 
  security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
  security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 29fc78a 
  ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java a4b12b2 
  unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
  unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 4b56936 

Diff: https://reviews.apache.org/r/55669/diff/


Testing
-------


Thanks,

Colm O hEigeartaigh


Re: Review Request 55669: RANGER-1318 - Consolidate XML configuration parsing

Posted by Sailaja Polavarapu <sp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55669/#review164609
-----------------------------------------------------------



Looks like there are recent changes to security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java because of which I am not able to apply the patch. Once I fixed that, I compiled and tried the changes in one of my test cluster with usersync module configuration changes. I was able to test the changes successfully with usersync.

Also, I went through the changes and they look fine. Please sync with latest changes and update the patch.

- Sailaja Polavarapu


On Feb. 1, 2017, 5:47 p.m., Colm O hEigeartaigh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55669/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 5:47 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1318
>     https://issues.apache.org/jira/browse/RANGER-1318
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Currently, duplicate code appears in several places to parse XML. Instead it should all be consolidated in a single place.
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/ranger/plugin/util/XMLUtils.java PRE-CREATION 
>   embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java 54c190d 
>   security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java cb8b825 
>   security-admin/src/main/java/org/apache/ranger/common/XMLPropertiesUtil.java 8fd6196 
>   ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824 
>   unixauthclient/src/main/java/org/apache/ranger/authentication/unix/jaas/RemoteUnixLoginModule.java 178eb53 
>   unixauthservice/src/main/java/org/apache/ranger/authentication/UnixAuthenticationService.java 4b56936 
> 
> Diff: https://reviews.apache.org/r/55669/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>