Re: Review Request 49392: SENTRY-1324: Add sentry specific test cases to use NotificationLog using DbNotificationListener

[Hao Hao]

> On June 30, 2016, 11:56 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestHMSNotificationLogUsingDBNotificationListener.java,
> >  line 61
> > 
> >
> > Should we also consider a concurrent test case which have multiple hms 
> > clients?
> 
> Sravya Tirukkovalur wrote:
> Yeah, I am mostly focusing on functional tests. Shall I create a follow 
> on jira?

Sure, thanks!


- Hao


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


On July 1, 2016, 11:43 p.m., Sravya Tirukkovalur wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49392/
> ---
> 
> (Updated July 1, 2016, 11:43 p.m.)
> 
> 
> Review request for sentry, Anne Yu, Colin McCabe, and Hao Hao.
> 
> 
> Bugs: Sentry-1324
> https://issues.apache.org/jira/browse/Sentry-1324
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Add sentry specific test cases to use NotificationLog using 
> DbNotificationListener
> 
> 
> Diffs
> -
> 
>   sentry-tests/sentry-tests-hive/pom.xml 
> 02bfa49c9513a36a93017c706375bf58d063d7d7 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  2add2d06ad1a54cf6746ac2e8d71ef3bc67899f7 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java
>  f1e6d75b2ae8e20343ff49bd46ef254ed6320ad4 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestHMSNotificationLogUsingDBNotificationListener.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49392/diff/
> 
> 
> Testing
> ---
> 
> All tests: new and regression pass.
> 
> 
> Thanks,
> 
> Sravya Tirukkovalur
> 
>

Re: Review Request 49392: SENTRY-1324: Add sentry specific test cases to use NotificationLog using DbNotificationListener

[Sravya Tirukkovalur]

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

(Updated July 1, 2016, 11:43 p.m.)


Review request for sentry, Anne Yu, Colin McCabe, and Hao Hao.


Changes
---

Split the test cases into multiple smaller test functions. And addressed other 
comments from Hao. Thanks for the review Hao!


Bugs: Sentry-1324
https://issues.apache.org/jira/browse/Sentry-1324


Repository: sentry


Description
---

Add sentry specific test cases to use NotificationLog using 
DbNotificationListener


Diffs (updated)
-

  sentry-tests/sentry-tests-hive/pom.xml 
02bfa49c9513a36a93017c706375bf58d063d7d7 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 2add2d06ad1a54cf6746ac2e8d71ef3bc67899f7 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java
 f1e6d75b2ae8e20343ff49bd46ef254ed6320ad4 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestHMSNotificationLogUsingDBNotificationListener.java
 PRE-CREATION 

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


Testing
---

All tests: new and regression pass.


Thanks,

Sravya Tirukkovalur

Re: Review Request 49392: SENTRY-1324: Add sentry specific test cases to use NotificationLog using DbNotificationListener

[Sravya Tirukkovalur]

> On June 30, 2016, 11:56 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestHMSNotificationLogUsingDBNotificationListener.java,
> >  line 61
> > 
> >
> > Should we also consider a concurrent test case which have multiple hms 
> > clients?

Yeah, I am mostly focusing on functional tests. Shall I create a follow on jira?


- Sravya


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


On June 29, 2016, 7:56 p.m., Sravya Tirukkovalur wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49392/
> ---
> 
> (Updated June 29, 2016, 7:56 p.m.)
> 
> 
> Review request for sentry, Anne Yu, Colin McCabe, and Hao Hao.
> 
> 
> Bugs: Sentry-1324
> https://issues.apache.org/jira/browse/Sentry-1324
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Add sentry specific test cases to use NotificationLog using 
> DbNotificationListener
> 
> 
> Diffs
> -
> 
>   sentry-tests/sentry-tests-hive/pom.xml 
> 02bfa49c9513a36a93017c706375bf58d063d7d7 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  2add2d06ad1a54cf6746ac2e8d71ef3bc67899f7 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java
>  f1e6d75b2ae8e20343ff49bd46ef254ed6320ad4 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestHMSNotificationLogUsingDBNotificationListener.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49392/diff/
> 
> 
> Testing
> ---
> 
> All tests: new and regression pass.
> 
> 
> Thanks,
> 
> Sravya Tirukkovalur
> 
>

Re: Review Request 49526: SENTRY-1365

[Hao Hao]

> On July 1, 2016, 6:57 p.m., Anne Yu wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql,
> >  line 1
> > 
> >
> > Need to add this file into 
> > "https://github.com/apache/sentry/blob/master/sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-derby-1.7.0-to-1.8.0.sql";
> >  to get it run for upgrade? The same for other 007-SENTRY-872.db.sql files.

Sure, will add it.


> On July 1, 2016, 6:57 p.m., Anne Yu wrote:
> > sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql,
> >  line 12
> > 
> >
> > Since it's transational db, is it possible to have 3 tables?
> > 
> > 1. AUTHZ (AUTHZ_OBJ_ID, AUTHZ_OBJ_ID, CREATE_TIME_MS)
> > 2. MAUTHZPATHSMAPPING_PATHS(PATH_ID, PATHS); not sure if it's better 
> > for one path to have 1 row with 1 id; or a group of paths to have 1 row 
> > with 1 id, whichever maps the most privileges.
> > 3. AUTHZ_PATHS_MAPPING(AUTHZ_OBJ_ID, PATH_ID); and corresponding 
> > foreign keys.

This is the auto generated sql from datanuclues conceptual table. There is only 
one logic table(MAuthzPathsMapping) defined which contains authzObj and 
Set. Datanuclues created the join table (MAUTHZPATHSMAPPING_PATHS) to 
hold the collection elements. For 
reference,http://www.datanucleus.org/products/accessplatform/jdo/orm/one_to_many_collection.html#join_nonpc


- Hao


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


On July 1, 2016, 6:20 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49526/
> ---
> 
> (Updated July 1, 2016, 6:20 p.m.)
> 
> 
> Review request for sentry and Anne Yu.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1365: Upgrading SQL script for HMSPaths persistence
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.mysql.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.oracle.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.postgres.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-1.8.0.sql
>  927f302f789913a787b177b89af8b90a23c013ca 
> 
> Diff: https://reviews.apache.org/r/49526/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 49526: SENTRY-1365

[Anne Yu]

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



Thanks Hao Hao. Left two comments here.


sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql 
(line 1)


Need to add this file into 
"https://github.com/apache/sentry/blob/master/sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-derby-1.7.0-to-1.8.0.sql";
 to get it run for upgrade? The same for other 007-SENTRY-872.db.sql files.



sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql 
(line 12)


Since it's transational db, is it possible to have 3 tables?

1. AUTHZ (AUTHZ_OBJ_ID, AUTHZ_OBJ_ID, CREATE_TIME_MS)
2. MAUTHZPATHSMAPPING_PATHS(PATH_ID, PATHS); not sure if it's better for 
one path to have 1 row with 1 id; or a group of paths to have 1 row with 1 id, 
whichever maps the most privileges.
3. AUTHZ_PATHS_MAPPING(AUTHZ_OBJ_ID, PATH_ID); and corresponding foreign 
keys.


- Anne Yu


On July 1, 2016, 6:20 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49526/
> ---
> 
> (Updated July 1, 2016, 6:20 p.m.)
> 
> 
> Review request for sentry and Anne Yu.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1365: Upgrading SQL script for HMSPaths persistence
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.mysql.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.oracle.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.postgres.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-1.8.0.sql
>  927f302f789913a787b177b89af8b90a23c013ca 
> 
> Diff: https://reviews.apache.org/r/49526/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Review Request 49526: SENTRY-1365

[Hao Hao]

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

Review request for sentry and Anne Yu.


Repository: sentry


Description
---

SENTRY-1365: Upgrading SQL script for HMSPaths persistence


Diffs
-

  
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.derby.sql 
PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.mysql.sql 
PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.oracle.sql 
PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/resources/007-SENTRY-872.postgres.sql
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-1.8.0.sql
 927f302f789913a787b177b89af8b90a23c013ca 

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


Testing
---


Thanks,

Hao Hao

Review Request 49522: SENTRY-1378: MAde changes to a make allowed sentry users to be case sensitive.

[Rahul Sharma]

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

Review request for sentry and Sravya Tirukkovalur.


Repository: sentry


Description
---

SENTRY-1378: Login fails for a secure Sentry Web UI
Made changes for users to be case sensitive


Diffs
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAuthFilter.java
 c1cfc1b7cd2bc3f6c74cbe95b9d7b0d58a208408 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryWebServerWithKerberos.java
 ece2ee83886bf86f028783841fcef47a40f9b0ad 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
 cb2d9c9d57e53c40096d6767213212edd829c0f9 

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


Testing
---

Added a case.


Thanks,

Rahul Sharma

Re: Review Request 49491: SENTRY-1334 test and add test for CTAS and Create View AS SELECT (cross databases cases)

[Anne Yu]

> On July 1, 2016, 5:31 p.m., Anne Yu wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java,
> >  line 633
> > 
> >
> > Can you also try to add one test for Create View AS SELECT;
> > 
> > create table db_1.tb1(int id, string val, int num);
> > ...
> > 
> > use db_1;
> > create view db_1.v1 as select db_1.tb1.id, db_2.tb1.num, db_2.tb2.val 
> > from  db_1.tb1, db_2.tb1, db_2.tb2; (the syntax might be incorrect please 
> > modify as needed)

Can you also insert some data into each of tables and select * from view to 
assert data are correctly inserted. Can this test case be also run in 
dbprovider profile on a real cluster?


- Anne


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


On July 1, 2016, 5:15 a.m., Ke Jia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49491/
> ---
> 
> (Updated July 1, 2016, 5:15 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Dapeng Sun.
> 
> 
> Bugs: SENTRY-1334
> https://issues.apache.org/jira/browse/SENTRY-1334
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> test and add test for CTAS and Create View AS SELECT (cross databases cases)
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java
>  b123dcd 
> 
> Diff: https://reviews.apache.org/r/49491/diff/
> 
> 
> Testing
> ---
> 
> test done
> 
> 
> Thanks,
> 
> Ke Jia
> 
>

Re: Review Request 49491: SENTRY-1334 test and add test for CTAS and Create View AS SELECT (cross databases cases)

[Anne Yu]

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




sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java
 (line 633)


Can you also try to add one test for Create View AS SELECT;

create table db_1.tb1(int id, string val, int num);
...

use db_1;
create view db_1.v1 as select db_1.tb1.id, db_2.tb1.num, db_2.tb2.val from  
db_1.tb1, db_2.tb1, db_2.tb2; (the syntax might be incorrect please modify as 
needed)


- Anne Yu


On July 1, 2016, 5:15 a.m., Ke Jia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49491/
> ---
> 
> (Updated July 1, 2016, 5:15 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Dapeng Sun.
> 
> 
> Bugs: SENTRY-1334
> https://issues.apache.org/jira/browse/SENTRY-1334
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> test and add test for CTAS and Create View AS SELECT (cross databases cases)
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java
>  b123dcd 
> 
> Diff: https://reviews.apache.org/r/49491/diff/
> 
> 
> Testing
> ---
> 
> test done
> 
> 
> Thanks,
> 
> Ke Jia
> 
>