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

Re: Review Request 61264: SENTRY-1843: Fork sentry-binding-hive-follower package to support Hive 2.x

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



I'm not sure the datanucleus properties are correct in the v2 follower module. The bindings module for hive v2 has different (more recent) versions.
Also in the "sentry-provider-db" pom, change the "versions" from "2.0.0-SNAPSHOT" to "${project.version}" (sentry-binding-hive-conf + the two hive follower dependencies).

- Colm O hEigeartaigh


On July 31, 2017, 8:10 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61264/
> -----------------------------------------------------------
> 
> (Updated July 31, 2017, 8:10 p.m.)
> 
> 
> Review request for sentry and Colm O hEigeartaigh.
> 
> 
> Bugs: SENTRY-1843
>     https://issues.apache.org/jira/browse/SENTRY-1843
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Message definition for the MessageFactory defined in org.apache.hive.hcatalog package is changes in Hive 2.x. We need to have a different for sentry-binding-hive-follower for hive 2.x.
> 
> we need to maintain support for both for Hive 1.1.0 and Hive 2.x until all the test failures with 2.x are properly handled. 
> when we know support for 2.x is stable, we can change the default dependency on hive to Hive 2.x and eventually remove support for hive 1.1.0. When this happens we will not have duplicate code.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/pom.xml 71fe47c 
>   sentry-binding/sentry-binding-hive-follower-v2/pom.xml PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/pom.xml 7b13cea 
> 
> 
> Diff: https://reviews.apache.org/r/61264/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>


Re: Review Request 61264: SENTRY-1843: Fork sentry-binding-hive-follower package to support Hive 2.x

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

> On Aug. 1, 2017, 9:50 a.m., Colm O hEigeartaigh wrote:
> > I'm not sure the datanucleus properties are correct in the v2 follower module. The bindings module for hive v2 has different (more recent) versions.
> > Also in the "sentry-provider-db" pom, change the "versions" from "2.0.0-SNAPSHOT" to "${project.version}" (sentry-binding-hive-conf + the two hive follower dependencies).
> 
> kalyan kumar kalvagadda wrote:
>     1. v2 follower may not need this datanucleus dependencies in the first place. I will make an effort to validate the dependecy and remove it if not needed in seperate jira. 
>     
>     For now I have addressed your comments in my new patch.

Could you update this RR as well with the latest patch? I notice the new files seem to use 4 space indentation - isn't the convention of the project to use 2 spaces?


- Colm


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


On July 31, 2017, 8:10 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61264/
> -----------------------------------------------------------
> 
> (Updated July 31, 2017, 8:10 p.m.)
> 
> 
> Review request for sentry and Colm O hEigeartaigh.
> 
> 
> Bugs: SENTRY-1843
>     https://issues.apache.org/jira/browse/SENTRY-1843
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Message definition for the MessageFactory defined in org.apache.hive.hcatalog package is changes in Hive 2.x. We need to have a different for sentry-binding-hive-follower for hive 2.x.
> 
> we need to maintain support for both for Hive 1.1.0 and Hive 2.x until all the test failures with 2.x are properly handled. 
> when we know support for 2.x is stable, we can change the default dependency on hive to Hive 2.x and eventually remove support for hive 1.1.0. When this happens we will not have duplicate code.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/pom.xml 71fe47c 
>   sentry-binding/sentry-binding-hive-follower-v2/pom.xml PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/pom.xml 7b13cea 
> 
> 
> Diff: https://reviews.apache.org/r/61264/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>


Re: Review Request 61264: SENTRY-1843: Fork sentry-binding-hive-follower package to support Hive 2.x

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.

> On Aug. 1, 2017, 9:50 a.m., Colm O hEigeartaigh wrote:
> > I'm not sure the datanucleus properties are correct in the v2 follower module. The bindings module for hive v2 has different (more recent) versions.
> > Also in the "sentry-provider-db" pom, change the "versions" from "2.0.0-SNAPSHOT" to "${project.version}" (sentry-binding-hive-conf + the two hive follower dependencies).
> 
> kalyan kumar kalvagadda wrote:
>     1. v2 follower may not need this datanucleus dependencies in the first place. I will make an effort to validate the dependecy and remove it if not needed in seperate jira. 
>     
>     For now I have addressed your comments in my new patch.
> 
> Colm O hEigeartaigh wrote:
>     Could you update this RR as well with the latest patch? I notice the new files seem to use 4 space indentation - isn't the convention of the project to use 2 spaces?

Even the old file in sentry-binding-hive-follower has 4 space indentation. That is why new file are have  wrong indentation. Will fix all of them


- kalyan kumar


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


On July 31, 2017, 8:10 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61264/
> -----------------------------------------------------------
> 
> (Updated July 31, 2017, 8:10 p.m.)
> 
> 
> Review request for sentry and Colm O hEigeartaigh.
> 
> 
> Bugs: SENTRY-1843
>     https://issues.apache.org/jira/browse/SENTRY-1843
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Message definition for the MessageFactory defined in org.apache.hive.hcatalog package is changes in Hive 2.x. We need to have a different for sentry-binding-hive-follower for hive 2.x.
> 
> we need to maintain support for both for Hive 1.1.0 and Hive 2.x until all the test failures with 2.x are properly handled. 
> when we know support for 2.x is stable, we can change the default dependency on hive to Hive 2.x and eventually remove support for hive 1.1.0. When this happens we will not have duplicate code.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/pom.xml 71fe47c 
>   sentry-binding/sentry-binding-hive-follower-v2/pom.xml PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java PRE-CREATION 
>   sentry-binding/sentry-binding-hive-follower-v2/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/pom.xml 7b13cea 
> 
> 
> Diff: https://reviews.apache.org/r/61264/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>