Review Request 45221: Add a test run to exclude slow and not thread safe tests. It could be used for pre-commit jobs

[Anne Yu]

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

Review request for sentry.


Bugs: SENTRY-1108
https://issues.apache.org/jira/browse/SENTRY-1108


Repository: sentry


Description
---

The way to run it: mvn test -P skipSlowAndNotThreadSafeTests; So it won't 
affect default mvn test;
Most excluded slow tests are from sentry-tests/e2e suites and a few are those 
can't be run under concurrent mode.


Diffs
-

  pom.xml d25c314e0a4c92ea03f22c01c07e0b0beba2bbea 

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


Testing
---

Test the running time on an ec2 cluster (large). Run mvn clean install first 
then mvn test.

Full suite: 2039 tests, 92m58.727s;
skipSlowAndNotThreadSafeTests run: 1027 tests, 6m47.195s;


Thanks,

Anne Yu

Re: Sonar build

[Anne Yu]

Hi Colm,

Could it be possible to have code coverage enabled from Sonar? Searched
through the linked pages, couldn't find any. It would be very helpful to
have sonar report with coverage numbers.

Thanks in advance,
Anne

On Wed, Dec 16, 2015 at 9:02 AM, Colm O hEigeartaigh 
wrote:

> Hi all,
>
> Sentry is now included in analysis.apache.org:
>
> https://analysis.apache.org/dashboard/index/246756
>
> Colm.
>
>
> --
> Colm O hEigeartaigh
>
> Talend Community Coder
> http://coders.talend.com
>



-- 
Anne

Re: Sonar build

[Anne Yu]

Sounds a good idea. I will do some investigations. But seems we have apache
clover license, which could be used instead.

Thanks,
Anne

On Tue, Mar 29, 2016 at 8:05 AM, Colm O hEigeartaigh 
wrote:

> Hi Anne,
>
> I don't know how to get the code coverage displayed - I agree that it would
> be useful. Perhaps drop INFRA a mail to ask them if it possible to enable
> it?
>
> Colm.
>
> On Fri, Mar 25, 2016 at 11:31 PM, Anne Yu  wrote:
>
> > Hi Colm,
> >
> > Could it be possible to have code coverage enabled from Sonar? Searched
> > through the linked pages, couldn't find any. It would be very helpful to
> > have sonar report with coverage numbers.
> >
> > Thanks in advance,
> > Anne
> >
> > On Wed, Dec 16, 2015 at 9:02 AM, Colm O hEigeartaigh <
> cohei...@apache.org>
> > wrote:
> >
> > > Hi all,
> > >
> > > Sentry is now included in analysis.apache.org:
> > >
> > > https://analysis.apache.org/dashboard/index/246756
> > >
> > > Colm.
> > >
> > >
> > > --
> > > Colm O hEigeartaigh
> > >
> > > Talend Community Coder
> > > http://coders.talend.com
> > >
> >
> >
> >
> > --
> > Anne
> >
>
>
>
> --
> Colm O hEigeartaigh
>
> Talend Community Coder
> http://coders.talend.com
>



-- 
Anne

Review Request 45502: SENTRY-1164: fix one e2e test failure on a real cluster; improved concurrent tests to be able to run locally.

[Anne Yu]

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

Review request for sentry and Hao Hao.


Bugs: sentry-1164
https://issues.apache.org/jira/browse/sentry-1164


Repository: sentry


Description
---

1. Fix one test failure in 
org.apache.sentry.tests.e2e.dbprovider.TestDbOperations.testCaseSensitivity;
2. Improve TestConcurrentClients to be able to run locally; also add more 
validation logics.


Diffs
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java
 bf871f01a8cdc0bab3e502c4fbdbee9285b8437b 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 b96175797dfc8bf238722b6177a7800d569b7e8a 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
 7d3db2b135d3239410bb664dbf7aed5817ee01c0 

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


Testing
---

Passed precommit build.


Thanks,

Anne Yu

Re: [jira] [Commented] (SENTRY-514) Enable e2e tests for authorization V2

[Anne Yu]

Ic, we need to think of a way to align tests with checkins instead of
running everthing. But this is not the scope of this jira. And it's good to
know by default we are not running v2 tests.
On Mar 30, 2016 6:27 PM, "Dapeng Sun (JIRA)"  wrote:

>
> [
> https://issues.apache.org/jira/browse/SENTRY-514?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15219166#comment-15219166
> ]
>
> Dapeng Sun commented on SENTRY-514:
> ---
>
> Hi [~anneyu], by default  these v2 tests will not be executed, we need to
> enable the profile  {-P-hive-authz1,hive-authz2,-datanucleus3,datanucleus4}
> for v2 binding and tests, I think {{3 hours}} may cause by there are so
> many componments: Hive, Solr, Kafka and Sqoop...
>
> > Enable e2e tests for authorization V2
> > -
> >
> > Key: SENTRY-514
> > URL: https://issues.apache.org/jira/browse/SENTRY-514
> > Project: Sentry
> >  Issue Type: Sub-task
> >  Components: Hive V2
> >Reporter: Xiaomeng Huang
> >Assignee: Dapeng Sun
> > Attachments: SENTRY-514.001.patch, SENTRY-514.002.patch
> >
> >
> > In V2, we add metadata check before grant/revoke task. So we may need to
> change some e2e tests.
> > If the table is not existing, we should create this table at first.
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)
>

Re: Review Request 45502: SENTRY-1164: fix one e2e test failure on a real cluster; improved concurrent tests to be able to run locally.

[Anne Yu]

> On March 31, 2016, 7:06 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java,
> >  line 323
> > <https://reviews.apache.org/r/45502/diff/1/?file=1319887#file1319887line323>
> >
> > Will the URI scheme be changed later? Should we get it from the 
> > filesystem config?

yeah, will change this line to make it flexible.


> On March 31, 2016, 7:06 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java,
> >  line 1098
> > <https://reviews.apache.org/r/45502/diff/1/?file=1319889#file1319889line1098>
> >
> > Why add scheme again if the dir starts with s3?

There is a not in the front. So if there is no scheme in the front will add it.


- Anne


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


On March 30, 2016, 7:20 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45502/
> ---
> 
> (Updated March 30, 2016, 7:20 p.m.)
> 
> 
> Review request for sentry and Hao Hao.
> 
> 
> Bugs: sentry-1164
> https://issues.apache.org/jira/browse/sentry-1164
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> 1. Fix one test failure in 
> org.apache.sentry.tests.e2e.dbprovider.TestDbOperations.testCaseSensitivity;
> 2. Improve TestConcurrentClients to be able to run locally; also add more 
> validation logics.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java
>  bf871f01a8cdc0bab3e502c4fbdbee9285b8437b 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  b96175797dfc8bf238722b6177a7800d569b7e8a 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
>  7d3db2b135d3239410bb664dbf7aed5817ee01c0 
> 
> Diff: https://reviews.apache.org/r/45502/diff/
> 
> 
> Testing
> ---
> 
> Passed precommit build.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 45502: SENTRY-1164: fix one e2e test failure on a real cluster; improved concurrent tests to be able to run locally.

[Anne Yu]

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

(Updated March 31, 2016, 11:19 p.m.)


Review request for sentry and Hao Hao.


Changes
---

Addressed Haohao's comments.


Bugs: sentry-1164
https://issues.apache.org/jira/browse/sentry-1164


Repository: sentry


Description
---

1. Fix one test failure in 
org.apache.sentry.tests.e2e.dbprovider.TestDbOperations.testCaseSensitivity;
2. Improve TestConcurrentClients to be able to run locally; also add more 
validation logics.


Diffs (updated)
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java
 bf871f01a8cdc0bab3e502c4fbdbee9285b8437b 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 b96175797dfc8bf238722b6177a7800d569b7e8a 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java
 7d3db2b135d3239410bb664dbf7aed5817ee01c0 

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


Testing
---

Passed precommit build.


Thanks,

Anne Yu

Re: Review Request 45221: Add a test run to exclude slow and not thread safe tests. It could be used for pre-commit jobs

[Anne Yu]

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

(Updated April 1, 2016, 12:03 a.m.)


Review request for sentry and Hao Hao.


Changes
---

Trim more carefully the excluded tests. Totall running time is 15min; However 
code coverage for core sentry packages is 63.2 (before trim, it is 75%).


Bugs: SENTRY-1108
https://issues.apache.org/jira/browse/SENTRY-1108


Repository: sentry


Description
---

The way to run it: mvn test -P skipSlowAndNotThreadSafeTests; So it won't 
affect default mvn test;
Most excluded slow tests are from sentry-tests/e2e suites and a few are those 
can't be run under concurrent mode.


Diffs (updated)
-

  pom.xml 37db0079503a5a3806526573904a989688a349fe 

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


Testing
---

Test the running time on an ec2 cluster (large). Run mvn clean install first 
then mvn test.

Full suite: 2039 tests, 92m58.727s;
skipSlowAndNotThreadSafeTests run: 1027 tests, 6m47.195s;


Thanks,

Anne Yu

Re: Review Request 45153: SENTRY-1155: Add waiting time for getMetastoreClient for avoiding metastore isn't ready

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On March 22, 2016, 10:55 a.m., Dapeng Sun wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45153/
> ---
> 
> (Updated March 22, 2016, 10:55 a.m.)
> 
> 
> Review request for sentry and Colin Ma.
> 
> 
> Bugs: SENTRY-1155
> https://issues.apache.org/jira/browse/SENTRY-1155
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> For TestMetastoreEndToEnd, If metastore didn't finish the initialization. 
> context.getMetaStoreClient will throw exception.
> HiveMetaStoreClient client = context.getMetaStoreClient(ADMIN1);
> We should wait for the initialization of metastore, for example:
> HiveMetaStoreClient client = null;
> for (int i=0; i < 10; i++) {
>   try {
> client = context.getMetaStoreClient(ADMIN1);
> break;
>   } catch (Throwable e) {
> // ignore
>   }
>   Thread.sleep(6000);
> }
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
>  2e508d6 
> 
> Diff: https://reviews.apache.org/r/45153/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dapeng Sun
> 
>

Re: Review Request 45152: SENTRY-1156: TestDbColumnLevelMetaDataOps should add `use database` for user session created

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On March 22, 2016, 7:38 a.m., Dapeng Sun wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45152/
> ---
> 
> (Updated March 22, 2016, 7:38 a.m.)
> 
> 
> Review request for sentry and Colin Ma.
> 
> 
> Bugs: SENTRY-1156
> https://issues.apache.org/jira/browse/SENTRY-1156
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> for the Tests like TestDbColumnLevelMetaDataOps#testShowNewColumn()
> establishSession(USER1_1);
> statement.execute("USE " + TEST_COL_METADATA_OPS_DB);
> We should add statement.execute("USE " + TEST_COL_METADATA_OPS_DB); before 
> executing queries...
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbColumnLevelMetaDataOps.java
>  a454202 
> 
> Diff: https://reviews.apache.org/r/45152/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dapeng Sun
> 
>

Re: Sentry download links are all invalid

[Anne Yu]

Hi Tuong,

Can you list the invalid links? We are in the process of moving from
incubator to TLP.

Thanks for reporting the issue,
Anne

On Wed, Apr 6, 2016 at 8:27 AM, Tuong Truong  wrote:

> Hi,
>
> Just notice that all the links for downloading various release of Sentry
> are invalids including all the mirror I tried.  Is is a side effect of the
> TLP status changes and will be fixed auto-magically?
>
> Tuong
>



-- 
Anne

PreCommit-SENTRY-Build still compiles incubating codes

[Anne Yu]

Hi,

Just found out "PreCommit-SENTRY-Build" still compiling incubator-sentry
repo so keep getting compiling errors. Can someone help fix it?

https://builds.apache.org/job/PreCommit-SENTRY-Build/1437/artifact/test-output/install.txt

[INFO] Installing
/home/jenkins/jenkins-slave/workspace/PreCommit-SENTRY-Build/sentry-core/pom.xml
to 
/home/jenkins/jenkins-slave/workspace/PreCommit-SENTRY-Build/maven-repo/org/apache/sentry/sentry-core/1.7.0-incubating-SNAPSHOT/sentry-core-1.7.0-incubating-SNAPSHOT.pom
[INFO]
[INFO] 
[INFO] Building Sentry Core Common 1.7.0-incubating-SNAPSHOT
[INFO] 


Thanks,

-- 
Anne

Re: Review Request 45221: Add a test run to exclude slow and not thread safe tests. It could be used for pre-commit jobs

[Anne Yu]

> On April 9, 2016, 5:53 a.m., Hao Hao wrote:
> > pom.xml, line 900
> > <https://reviews.apache.org/r/45221/diff/2/?file=1321664#file1321664line900>
> >
> > For a Quad-Core system, if each VM has max heap size to be 2G, and 10 
> > threads will be running simultaneously, so the RAM should be 20G at least? 
> > Is it safe to assume that?

Thanks for reviewing this patch.

-Xmx2g is the maximum heap size could be available for the process; but I think 
it's a valid point. May decrease -Xms512m to -Xms256m; The main test 
infrastructure I used is pre-commit job machines.


> On April 9, 2016, 5:53 a.m., Hao Hao wrote:
> > pom.xml, line 908
> > <https://reviews.apache.org/r/45221/diff/2/?file=1321664#file1321664line908>
> >
> > Should we set reuseForks to true, so that to save resource/time for 
> > creating a new process for testing every time one test class is done?

Can't. For some tests, if context in one JVM is not cleared up completely, will 
cause unexpected failures. I've seen failures. But even if reuseForks is false, 
tests still finish quite fast; reuse or not-reuse doesn't gain much.


> On April 9, 2016, 5:53 a.m., Hao Hao wrote:
> > pom.xml, line 917
> > <https://reviews.apache.org/r/45221/diff/2/?file=1321664#file1321664line917>
> >
> > Why do we need to exclude TestURI?

It requires dedicated resouces can't share with other JVM processes. As far as 
why, need to dig more.


- Anne


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


On April 1, 2016, 12:03 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45221/
> ---
> 
> (Updated April 1, 2016, 12:03 a.m.)
> 
> 
> Review request for sentry and Hao Hao.
> 
> 
> Bugs: SENTRY-1108
> https://issues.apache.org/jira/browse/SENTRY-1108
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The way to run it: mvn test -P skipSlowAndNotThreadSafeTests; So it won't 
> affect default mvn test;
> Most excluded slow tests are from sentry-tests/e2e suites and a few are those 
> can't be run under concurrent mode.
> 
> 
> Diffs
> -
> 
>   pom.xml 37db0079503a5a3806526573904a989688a349fe 
> 
> Diff: https://reviews.apache.org/r/45221/diff/
> 
> 
> Testing
> ---
> 
> Test the running time on an ec2 cluster (large). Run mvn clean install first 
> then mvn test.
> 
> Full suite: 2039 tests, 92m58.727s;
> skipSlowAndNotThreadSafeTests run: 1027 tests, 6m47.195s;
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 45221: Add a test run to exclude slow and not thread safe tests. It could be used for pre-commit jobs

[Anne Yu]

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

(Updated April 12, 2016, 11:01 p.m.)


Review request for sentry and Hao Hao.


Changes
---

Addressed code comments regarding xms.


Bugs: SENTRY-1108
https://issues.apache.org/jira/browse/SENTRY-1108


Repository: sentry


Description
---

The way to run it: mvn test -P skipSlowAndNotThreadSafeTests; So it won't 
affect default mvn test;
Most excluded slow tests are from sentry-tests/e2e suites and a few are those 
can't be run under concurrent mode.


Diffs (updated)
-

  pom.xml c3754ed64d4e361024e4d8910e6d1f2ce86f1be7 

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


Testing
---

Test the running time on an ec2 cluster (large). Run mvn clean install first 
then mvn test.

Full suite: 2039 tests, 92m58.727s;
skipSlowAndNotThreadSafeTests run: 1027 tests, 6m47.195s;


Thanks,

Anne Yu

RE: Proposal for Apr Monthly Hangout

[Anne Yu]

Hi Team,

On Thursday either after 8pm or 10am works for me. 6pm will be late for
commuting home.

Thanks,
Anne
On Apr 13, 2016 12:49 AM, "Ma, Junjie"  wrote:

> Hi Hao,
>
> 9:00 am or later is ok for me if possible, thanks for understanding.
>
> Best regards,
>
> Colin Ma(Ma Jun Jie)
>
> -Original Message-
> From: Hao Hao [mailto:hao@cloudera.com]
> Sent: Wednesday, April 13, 2016 3:45 PM
> To: dev@sentry.apache.org
> Subject: Re: Proposal for Apr Monthly Hangout
>
> Hi Colin,
>
> Thanks a lot for the proposal. It looks very good to me. I am not sure if
> 18:00 PDT is 08:00 am or 9:00 am for you? But I am fine with moving the
> meeting time. Anyone else has any thoughts about changing the meeting time?
>
> Best,
> Hao
>
> On Tue, Apr 12, 2016 at 9:59 PM, Ma, Junjie  wrote:
>
> > Hi Hao,
> >
> > For the monthly hangout, I think you're in Summer time, and the
> > previous time is not suitable for me, can we adjust the time to 18:00
> > (UTC-08:00) or later?
> >
> > For the topics, I've several recommendations:
> > 1. SENTRY-999 (Refactor the sentry to integrate with external
> > components
> > quickly) is finished and ready to be merged to master.
> > 2. SENTRY-785 (Allow export of sentry for a specific auth object) is
> > finished and ready to be merged to master.
> > 3. Discussion on rest API for Sentry.
> >
> > Best regards,
> >
> > Colin Ma(Ma Jun Jie)
> >
> > -Original Message-
> > From: Hao Hao [mailto:hao@cloudera.com]
> > Sent: Wednesday, April 13, 2016 2:39 AM
> > To: dev@sentry.apache.org
> > Subject: Proposal for Apr Monthly Hangout
> >
> > Hi all,
> >
> > We are going to have the Apr monthly hangout this week. Any
> > recommendations on the topics you want to cover? Thanks!
> >
> > Best,
> > Hao
> >
>

Re: FAQs?

[Anne Yu]

Hi Bhooshan,

Thanks for reporting this issue. If you have clear idea of what's going on,
since you've spent time debugging the code, please feel free to create a
jira (https://issues.apache.org/jira/browse/SENTRY/) then post a fix.
Committers will be very happy to do the code review.

Best,
Anne

On Wed, Apr 20, 2016 at 9:17 PM, Bhooshan Mogal 
wrote:

> Hi folks,
>
> I suddenly started seeing the following error in the Sentry Service today
> and ended up debugging it for a long time across Sentry, Thrift and my own
> code.
>
> 16/04/21 02:15:51 ERROR server.TThreadPoolServer: Thrift error occurred
> during processing of message.
> org.apache.thrift.protocol.TProtocolException: Missing version in
> readMessageBegin, old client?
> at
>
> org.apache.thrift.protocol.TBinaryProtocol.readMessageBegin(TBinaryProtocol.java:228)
> at
>
> org.apache.thrift.TMultiplexedProcessor.process(TMultiplexedProcessor.java:92)
> at
>
> org.apache.thrift.server.TThreadPoolServer$WorkerProcess.run(TThreadPoolServer.java:285)
> at
>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> at
>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at java.lang.Thread.run(Thread.java:745)
>
>
>
> It turns out that my (Sentry) client had sentry.service.security.mode set
> to *kerberos* while the (Sentry) server had it set to *none. *But the error
> message threw me off completely.
>
> I tried to see if there was a way we could catch this scenario and throw a
> better error, but its difficult because as either the client or server, its
> not possible to know what the other is configured as. However, is there an
> FAQs section where such issues can be documented?
>
>
> Best,
> Bhooshan
>
>
>
> --
> Bhooshan
>



-- 
Anne

Re: FAQs?

[Anne Yu]

Hi Bhooshan,

IC, FAQ session could be very helpful. We have a upstream space for all
sentry docs (https://cwiki.apache.org/confluence/display/SENTRY/Home). You
might want to add a dedicated page for FAQ.

Besides if "sentry.service.security.mode" is inconsistent between server
and client, the client APIs could be improved and surface such issues into
exception or log error, instead of wiki page. If you have any idea could
contribute to it. That's what I meant.

Thanks,
Anne

On Thu, Apr 21, 2016 at 10:43 AM, Bhooshan Mogal 
wrote:

> Thanks Anne. Like I said, I'm pretty sure what's going on. But it seems
> very unlikely that we could add a fix for this in Sentry, since the error
> occurs in the thrift communication between SentryService and
> SentryGenericServiceClientDefultImpl. To me, this seemed like a good
> candidate for some FAQs section we there is one on wiki - so its at least
> discoverable when someone does a search for this error with Sentry.
>
> -
> Bhooshan
>
> On Thu, Apr 21, 2016 at 10:27 AM, Anne Yu  wrote:
>
> > Hi Bhooshan,
> >
> > Thanks for reporting this issue. If you have clear idea of what's going
> on,
> > since you've spent time debugging the code, please feel free to create a
> > jira (https://issues.apache.org/jira/browse/SENTRY/) then post a fix.
> > Committers will be very happy to do the code review.
> >
> > Best,
> > Anne
> >
> > On Wed, Apr 20, 2016 at 9:17 PM, Bhooshan Mogal <
> bhooshan.mo...@gmail.com>
> > wrote:
> >
> > > Hi folks,
> > >
> > > I suddenly started seeing the following error in the Sentry Service
> today
> > > and ended up debugging it for a long time across Sentry, Thrift and my
> > own
> > > code.
> > >
> > > 16/04/21 02:15:51 ERROR server.TThreadPoolServer: Thrift error occurred
> > > during processing of message.
> > > org.apache.thrift.protocol.TProtocolException: Missing version in
> > > readMessageBegin, old client?
> > > at
> > >
> > >
> >
> org.apache.thrift.protocol.TBinaryProtocol.readMessageBegin(TBinaryProtocol.java:228)
> > > at
> > >
> > >
> >
> org.apache.thrift.TMultiplexedProcessor.process(TMultiplexedProcessor.java:92)
> > > at
> > >
> > >
> >
> org.apache.thrift.server.TThreadPoolServer$WorkerProcess.run(TThreadPoolServer.java:285)
> > > at
> > >
> > >
> >
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> > > at
> > >
> > >
> >
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> > > at java.lang.Thread.run(Thread.java:745)
> > >
> > >
> > >
> > > It turns out that my (Sentry) client had sentry.service.security.mode
> set
> > > to *kerberos* while the (Sentry) server had it set to *none. *But the
> > error
> > > message threw me off completely.
> > >
> > > I tried to see if there was a way we could catch this scenario and
> throw
> > a
> > > better error, but its difficult because as either the client or server,
> > its
> > > not possible to know what the other is configured as. However, is there
> > an
> > > FAQs section where such issues can be documented?
> > >
> > >
> > > Best,
> > > Bhooshan
> > >
> > >
> > >
> > > --
> > > Bhooshan
> > >
> >
> >
> >
> > --
> > Anne
> >
>
>
>
> --
> Bhooshan
>



-- 
Anne

Review Request 46542: SENTRY-583: Add boundary condition test coverage to HDFS synchronization test suite around max #of groups

[Anne Yu]

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

Review request for sentry, Hao Hao and Lenni Kuff.


Bugs: SENTRY-583
https://issues.apache.org/jira/browse/SENTRY-583


Repository: sentry


Description
---

Add one boundary HDFS SyncUp test to run on a real cluster;
Also add some basic APIs for createing more HDFS SyncUp tests.


Diffs
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 8515a2ba1fc08104a788ea054f2b871905359273 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
 e1881b4bfaa5923fd0dab2096f670b22b944f116 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java
 77af4329642807283e6cd3d2e3be837c5d3e0dfb 

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


Testing
---

Tested on a unmanaged real cluster.


 T E S T S
---
Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in 
org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0


Thanks,

Anne Yu

Re: Review Request 46542: SENTRY-583: Add boundary condition test coverage to HDFS synchronization test suite around max #of groups

[Anne Yu]

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

(Updated April 21, 2016, 11:47 p.m.)


Review request for sentry, Hao Hao and Lenni Kuff.


Changes
---

I also added one more test case to test one scenario when there is an external 
path configured in the prefix.


Bugs: SENTRY-583
https://issues.apache.org/jira/browse/SENTRY-583


Repository: sentry


Description
---

Add one boundary HDFS SyncUp test to run on a real cluster;
Also add some basic APIs for createing more HDFS SyncUp tests.


Diffs (updated)
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 8515a2ba1fc08104a788ea054f2b871905359273 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
 e1881b4bfaa5923fd0dab2096f670b22b944f116 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java
 77af4329642807283e6cd3d2e3be837c5d3e0dfb 

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


Testing
---

Tested on a unmanaged real cluster.


 T E S T S
---
Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in 
org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0


Thanks,

Anne Yu

Re: Review Request 46542: SENTRY-583: Add boundary condition test coverage to HDFS synchronization test suite around max #of groups

[Anne Yu]

> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java,
> >  line 124
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line124>
> >
> > What are the HDFS specif tearDown here?Will the tearDown be the same 
> > when enableHDFSAcls is true?

Previously thought about cleaning up all files/dirs under prefix path but 
forgot to add. Will add them in the next patch. Thanks for pointing it out.


> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java,
> >  line 173
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line173>
> >
> > Could you add the comments here to explain the retry logic for getting 
> > the acls? Also for the recursive verify logic?

It takes some time for acls to be synced up on hdfs files/dirs; Especially for 
these on external prefix path. On my testing cluster (2x large), takes >= 10s.


> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java,
> >  line 204
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line204>
> >
> > Commnet to explain why we need this?

In case the roles haven't be cleaned up correctly from previous runs. Don't 
want tests to fail because of it. Since most of the time, roles don't exit, 
might throw exception, so just put all these logic in a convenience method.


> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java,
> >  line 208
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line208>
> >
> > Log the exception here?

OK.


> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java,
> >  line 47
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356093#file1356093line47>
> >
> > Do we need to make the max_num_of_groups configurable for different 
> > value testing? e.g make it > 32?

Yeah, for table dir, it will have max_num_of_groups*2 acl entries; for 
partition dir, it will have max_num_of_groups*3 acl entries. More acl entries 
put into the dir, longer time to sync up acls across components. So I chose 20.


> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java,
> >  line 68
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356093#file1356093line68>
> >
> > How about test for external table outside of prefix? To check the ACLs 
> > are not changed by Sentry?

You meant negative test? Could add one for sure. Will do it in the next patch.


- Anne


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


On April 21, 2016, 11:47 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46542/
> ---
> 
> (Updated April 21, 2016, 11:47 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Lenni Kuff.
> 
> 
> Bugs: SENTRY-583
> https://issues.apache.org/jira/browse/SENTRY-583
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Add one boundary HDFS SyncUp test to run on a real cluster;
> Also add some basic APIs for createing more HDFS SyncUp tests.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  8515a2ba1fc08104a788ea054f2b871905359273 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
>  e1881b4bfaa5923fd0dab2096f670b22b944f116 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java
>  77af4329642807283e6cd3d2e3be837c5d3e0dfb 
> 
> Diff: https://reviews.apache.o

Re: Review Request 46542: SENTRY-583: Add boundary condition test coverage to HDFS synchronization test suite around max #of groups

[Anne Yu]

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

(Updated April 29, 2016, 8:44 p.m.)


Review request for sentry, Hao Hao and Lenni Kuff.


Changes
---

Addressed previous comments;
Added more validation to column level privileges and synced up acls;
Added negative test case;
Since they are boundary tests, splitted internal path tests and external path 
tests in different files; otherwise, test class runs long time.


Bugs: SENTRY-583
https://issues.apache.org/jira/browse/SENTRY-583


Repository: sentry


Description
---

Add one boundary HDFS SyncUp test to run on a real cluster;
Also add some basic APIs for createing more HDFS SyncUp tests.


Diffs (updated)
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 d2a1d36ee0572d5d24c200f631607e4cfa8168e3 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/PrivilegeResultSet.java
 8818c4c2b35b5a9979b7ca11a8a8548121bc348a 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
 e1881b4bfaa5923fd0dab2096f670b22b944f116 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java
 77af4329642807283e6cd3d2e3be837c5d3e0dfb 

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


Testing
---

Tested on a unmanaged real cluster.


 T E S T S
---
Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in 
org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0


Thanks,

Anne Yu

Re: Review Request 46957: Sentry-1234: JDO exception for list_sentry_privileges_by_authorizable

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On May 4, 2016, 1:58 a.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46957/
> ---
> 
> (Updated May 4, 2016, 1:58 a.m.)
> 
> 
> Review request for sentry and Anne Yu.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Sentry-1234: JDO exception for list_sentry_privileges_by_authorizable
> 
> Fix JDO exception for list_sentry_privileges_by_authorizable, and reorganized 
> the logic for list_sentry_privileges_by_authorizable.
> 
> Change-Id: Ia75e89b7d361d5ff659afe3f1d786fab99089e79
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  d51b3baf58de85633755598b55d78d6488fc5041 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
>  2a287e9b95e36b7d365de603d94208bad59966bc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java
>  60502895a0fcdd781f5bd61b29a676a7c96f81b8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java
>  dce3dade7f42fe35a849612c5caf2e98d2dac578 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java
>  fcf0e7b9dc45eb12cc1b76f3084efd6340c039a4 
> 
> Diff: https://reviews.apache.org/r/46957/diff/
> 
> 
> Testing
> ---
> 
> Added more test cases in TestSentryGenericServiceIntegration.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 46542: SENTRY-583: Add boundary condition test coverage to HDFS synchronization test suite around max #of groups

[Anne Yu]

> On May 4, 2016, 12:46 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java,
> >  line 69
> > <https://reviews.apache.org/r/46542/diff/3/?file=1367636#file1367636line69>
> >
> > Should we have similar test for MAX_NUM_OF_GROUPS > 20?

Let me just change this value to 33. Thanks Haohao.


- Anne


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


On April 29, 2016, 8:44 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46542/
> ---
> 
> (Updated April 29, 2016, 8:44 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Lenni Kuff.
> 
> 
> Bugs: SENTRY-583
> https://issues.apache.org/jira/browse/SENTRY-583
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Add one boundary HDFS SyncUp test to run on a real cluster;
> Also add some basic APIs for createing more HDFS SyncUp tests.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  d2a1d36ee0572d5d24c200f631607e4cfa8168e3 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/PrivilegeResultSet.java
>  8818c4c2b35b5a9979b7ca11a8a8548121bc348a 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
>  e1881b4bfaa5923fd0dab2096f670b22b944f116 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java
>  77af4329642807283e6cd3d2e3be837c5d3e0dfb 
> 
> Diff: https://reviews.apache.org/r/46542/diff/
> 
> 
> Testing
> ---
> 
> Tested on a unmanaged real cluster.
> 
> 
>  T E S T S
> ---
> Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - 
> in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
> 
> Results :
> 
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: [ANNOUNCE] New Sentry Committers - Li Li and Colm O hEigeartaigh

[Anne Yu]

Congrats Lili and Colm.
On May 6, 2016 10:26 AM, "Lenni Kuff"  wrote:

> I'm excited to announce that the Apache Sentry PMC has voted to invite Li
> Li and Colm O hEigeartaigh as committers on the Apache Sentry project.
>
> Please join me in congratulating Li and Colm!
>
> Thanks,
> Lenni
>

Re: [DISCUSS] Wiki updates on "How to integrate with Sentry"

[Anne Yu]

Hi Bhooshan,

https://cwiki.apache.org/confluence/display/SENTRY/Integrating+with+Sentry+New+Universal+Authorization+Model

Thanks,
Anne

On Wed, Mar 9, 2016 at 12:35 PM, Bhooshan Mogal 
wrote:

> Happy to review when the doc is available too. May have some feedback from
> the work we're currently doing to integrate with Sentry.
>
> -
> Bhooshan
>
> On Wed, Mar 9, 2016 at 11:53 AM, Sravya Tirukkovalur 
> wrote:
>
> > Awesome! Thanks Hao and Ashish!
> >
> > On Wed, Mar 9, 2016 at 11:40 AM, Ashish Singh 
> wrote:
> >
> > > Hey Guys,
> > >
> > > Makes a ton of sense to update the document. I can help with reviewing
> it
> > > and providing any feedbacks I might have.
> > >
> > > On Wed, Mar 9, 2016 at 11:38 AM, Hao Hao  wrote:
> > >
> > > > Hi Sravya,
> > > >
> > > > I feel we need to update the wiki with both the model design and
> > > > intergation example. I could try to put high level information there.
> > But
> > > > anyone has more detail information, please free feel to edit it.
> > Thanks!
> > > >
> > > > Best,
> > > > Hao
> > > >
> > > > On Wed, Mar 9, 2016 at 11:22 AM, Sravya Tirukkovalur <
> > > sra...@cloudera.com>
> > > > wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/SENTRY/Integration+with+Sentry
> > > > >
> > > > > Seems like we should update this wiki page with most recent
> > > developments
> > > > > with respect to Generic model and related enhancements. Any
> > volunteers?
> > > > > Some one who has done integration recently or some one doing
> > > enhancements
> > > > > can help here.
> > > > >
> > > > > Thanks a lot!
> > > > > --
> > > > > Sravya Tirukkovalur
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > >
> > > Regards,
> > > Ashish
> > >
> >
> >
> >
> > --
> > Sravya Tirukkovalur
> >
>
>
>
> --
> Bhooshan
>



-- 
Anne

Re: Review Request 45550: Fix some "major" issues identified by Sonarqube

[Anne Yu]

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


Fix it, then Ship it!





sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java
 (line 32)
<https://reviews.apache.org/r/45550/#comment196196>

Wondering which rule request this change? Generally it will be faster for 
review process if could add Sonarqube issues (links) into this cr. Or add some 
comments into important but not obviously change.



sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java
 (line 94)
<https://reviews.apache.org/r/45550/#comment196197>

Could add commets here?



sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/SentryPolicyFileFormatFactory.java
 (line 45)
<https://reviews.apache.org/r/45550/#comment196198>

Could add comments here?



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
 (line 307)
<https://reviews.apache.org/r/45550/#comment196204>

Trim white space.



sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/SentrySqoopError.java
 (line 32)
<https://reviews.apache.org/r/45550/#comment196205>

Trim white space.



sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/SentrySqoopError.java
 (line 34)
<https://reviews.apache.org/r/45550/#comment196207>

Still it would be nice to have a simple comment for why adding this class.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java 
(line 48)
<https://reviews.apache.org/r/45550/#comment196209>

Trim white space.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java 
(line 52)
<https://reviews.apache.org/r/45550/#comment196210>

Trim white space.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryVersionInfo.java
 (line 109)
<https://reviews.apache.org/r/45550/#comment196212>

White space.



sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/AccessConstants.java
 (line 44)
<https://reviews.apache.org/r/45550/#comment196213>

White space.



sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionConstant.java
 (line 25)
<https://reviews.apache.org/r/45550/#comment196215>

White space.



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
 (line 45)
<https://reviews.apache.org/r/45550/#comment196216>

White spaces and all the following places which needs trimming.


- Anne Yu


On May 6, 2016, 10:28 a.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45550/
> ---
> 
> (Updated May 6, 2016, 10:28 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1168: Fix some "major" issues identified by Sonarqube
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java
>  5238414 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java
>  f3799ca 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/SentryPolicyFileFormatFactory.java
>  d2c6072 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  775a1f5 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
>  4ef86e6 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
>  217b7b3 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
>  0d1db89 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
>  8e70492 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java
>  9e08571 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java
>  d741c44 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/SentrySqoopError.java
> 

Review Request 47682: SENTRY-1230: Add filesystem tests to test Sentry with user data storage on S3. The tests can also be run on other types of fileSystems (ms azure, hdfs). Just ensure pass correct

[Anne Yu]

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

Review request for sentry, Hao Hao, Lenni Kuff, and Sean Mackrory.


Bugs: SENTRY-1230
https://issues.apache.org/jira/browse/SENTRY-1230


Repository: sentry


Description
---

SENTRY-1230: Add filesystem tests to test Sentry with user data storage on S3. 
The tests can also be run on other types of fileSystems (ms azure, hdfs). Just 
ensure pass correct parameters.
For example,
   -Dsentry.e2etest.DFSType=S3DFS
   -Dsentry.e2etest.storage.uri=s3a://sentry-s3
   -Dfs.s3a.access.key=a
   -Dfs.s3a.secret.key=s
   export HIVE_CONF_DIR=/etc/hive/conf/hite-site.xml


Diffs
-

  pom.xml a82c66d9a730c76403600b12db7563522d6c12be 
  sentry-tests/sentry-tests-hive/pom.xml 
e36b5c04d48b3211abaecd36bf2673e6304bdc31 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
 001f5a49449112785cf129bfeda22c3bdf8cf3a6 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 0e4b3cab7685937e563b971274c66be43ba88fc1 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java
 32aabb41a1fef486ebed80571f0bd96e4c7c3c3c 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java
 06415cb828bfc6408bdc74d57c352ccfd55b7b43 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java
 67ba3386df6f52c1cd21d47eaec58a1a467bebcb 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
 7f650ceb01d6325c2dea794a207baaa5a4cb6e73 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/S3DFS.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSContants.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestHiveWarehouseOnExtFs.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestTableOnExtFS.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/AbstractHiveServer.java
 dda7dba662f7ba8fb9f705066c4bc680a7358f50 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/EmbeddedHiveServer.java
 52ba09e5e1006ff44a5073bce412ddee0774b24b 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServer.java
 175e84ceac8f24c1faa905d02daac1739e17f320 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java
 45f0ef28bc2393c16dd385cd8e37c133302b2993 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
 beae8e83bc9895f9c3aafd2eb9c6320b11ad0c1d 

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


Testing
---

---
 T E S T S
---
Running org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.419 sec - in 
org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

[INFO] 
[INFO] BUILD SUCCESS
[INFO] 
[INFO] Total time: 1:28.585s
[INFO] Finished at: Fri May 20 17:09:38 PDT 2016
[INFO] Final Memory: 60M/825M
[INFO] 


Thanks,

Anne Yu

Re: Review Request 47682: SENTRY-1230: Add filesystem tests to test Sentry with user data storage on S3. The tests can also be run on other types of fileSystems (ms azure, hdfs). Just ensure pass corr

[Anne Yu]

> On May 21, 2016, 5:58 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestHiveWarehouseOnExtFs.java,
> >  line 57
> > <https://reviews.apache.org/r/47682/diff/1/?file=1390156#file1390156line57>
> >
> > Is this test specific for s3 only?

Yeah, so far it's s3a. Once we can test Azure will add more conditions. Not 
sure that scheme will be used here. HDFS filesystem doesn't need to run this 
test since we already have so many in the dbprovider e2e.


> On May 21, 2016, 5:58 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestTableOnExtFS.java,
> >  line 73
> > <https://reviews.apache.org/r/47682/diff/1/?file=1390157#file1390157line73>
> >
> > Could you add some test for location URI without scheme and authority?

ok, will add one.


- Anne


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


On May 21, 2016, 12:23 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47682/
> ---
> 
> (Updated May 21, 2016, 12:23 a.m.)
> 
> 
> Review request for sentry, Hao Hao, Lenni Kuff, and Sean Mackrory.
> 
> 
> Bugs: SENTRY-1230
> https://issues.apache.org/jira/browse/SENTRY-1230
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1230: Add filesystem tests to test Sentry with user data storage on 
> S3. The tests can also be run on other types of fileSystems (ms azure, hdfs). 
> Just ensure pass correct parameters.
> For example,
>-Dsentry.e2etest.DFSType=S3DFS
>-Dsentry.e2etest.storage.uri=s3a://sentry-s3
>-Dfs.s3a.access.key=a
>-Dfs.s3a.secret.key=s
>export HIVE_CONF_DIR=/etc/hive/conf/hite-site.xml
> 
> 
> Diffs
> -
> 
>   pom.xml a82c66d9a730c76403600b12db7563522d6c12be 
>   sentry-tests/sentry-tests-hive/pom.xml 
> e36b5c04d48b3211abaecd36bf2673e6304bdc31 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
>  001f5a49449112785cf129bfeda22c3bdf8cf3a6 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  0e4b3cab7685937e563b971274c66be43ba88fc1 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java
>  32aabb41a1fef486ebed80571f0bd96e4c7c3c3c 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java
>  06415cb828bfc6408bdc74d57c352ccfd55b7b43 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java
>  67ba3386df6f52c1cd21d47eaec58a1a467bebcb 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
>  7f650ceb01d6325c2dea794a207baaa5a4cb6e73 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/S3DFS.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSContants.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestHiveWarehouseOnExtFs.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestTableOnExtFS.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/AbstractHiveServer.java
>  dda7dba662f7ba8fb9f705066c4bc680a7358f50 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/EmbeddedHiveServer.java
>  52ba09e5e1006ff44a5073bce412ddee0774b24b 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServer.java
>  175e84ceac8f24c1faa905d02daac1739e17f320 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java
>  45f0ef28bc2393c16dd385cd8e37c133302b2993 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
>  beae8e83bc9895f9c3aafd2eb9c6320b11ad0c1d 
> 
> Diff:

Re: Review Request 47682: SENTRY-1230: Add filesystem tests to test Sentry with user data storage on S3. The tests can also be run on other types of fileSystems (ms azure, hdfs). Just ensure pass corr

[Anne Yu]

> On May 21, 2016, 5:58 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestTableOnExtFS.java,
> >  line 73
> > <https://reviews.apache.org/r/47682/diff/1/?file=1390157#file1390157line73>
> >
> > Could you add some test for location URI without scheme and authority?
> 
> Anne Yu wrote:
> ok, will add one.

Just be noted, with current support s3a at runtime instead of defaultFS, this 
will actually test hdfs filesystem.


- Anne


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


On May 21, 2016, 12:23 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47682/
> ---
> 
> (Updated May 21, 2016, 12:23 a.m.)
> 
> 
> Review request for sentry, Hao Hao, Lenni Kuff, and Sean Mackrory.
> 
> 
> Bugs: SENTRY-1230
> https://issues.apache.org/jira/browse/SENTRY-1230
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1230: Add filesystem tests to test Sentry with user data storage on 
> S3. The tests can also be run on other types of fileSystems (ms azure, hdfs). 
> Just ensure pass correct parameters.
> For example,
>-Dsentry.e2etest.DFSType=S3DFS
>-Dsentry.e2etest.storage.uri=s3a://sentry-s3
>-Dfs.s3a.access.key=a
>-Dfs.s3a.secret.key=s
>export HIVE_CONF_DIR=/etc/hive/conf/hite-site.xml
> 
> 
> Diffs
> -
> 
>   pom.xml a82c66d9a730c76403600b12db7563522d6c12be 
>   sentry-tests/sentry-tests-hive/pom.xml 
> e36b5c04d48b3211abaecd36bf2673e6304bdc31 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
>  001f5a49449112785cf129bfeda22c3bdf8cf3a6 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  0e4b3cab7685937e563b971274c66be43ba88fc1 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java
>  32aabb41a1fef486ebed80571f0bd96e4c7c3c3c 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java
>  06415cb828bfc6408bdc74d57c352ccfd55b7b43 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java
>  67ba3386df6f52c1cd21d47eaec58a1a467bebcb 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
>  7f650ceb01d6325c2dea794a207baaa5a4cb6e73 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/S3DFS.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSContants.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestHiveWarehouseOnExtFs.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestTableOnExtFS.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/AbstractHiveServer.java
>  dda7dba662f7ba8fb9f705066c4bc680a7358f50 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/EmbeddedHiveServer.java
>  52ba09e5e1006ff44a5073bce412ddee0774b24b 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServer.java
>  175e84ceac8f24c1faa905d02daac1739e17f320 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java
>  45f0ef28bc2393c16dd385cd8e37c133302b2993 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
>  beae8e83bc9895f9c3aafd2eb9c6320b11ad0c1d 
> 
> Diff: https://reviews.apache.org/r/47682/diff/
> 
> 
> Testing
> ---
> 
> ---
>  T E S T S
> ---
> Running org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.419 sec - 
> in org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs
> 

Re: Review Request 47682: SENTRY-1230: Add filesystem tests to test Sentry with user data storage on S3. The tests can also be run on other types of fileSystems (ms azure, hdfs). Just ensure pass corr

[Anne Yu]

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

(Updated May 23, 2016, 8:56 p.m.)


Review request for sentry, Hao Hao, Lenni Kuff, and Sean Mackrory.


Changes
---

Addressed code review comment by adding a test case without scheme.


Bugs: SENTRY-1230
https://issues.apache.org/jira/browse/SENTRY-1230


Repository: sentry


Description
---

SENTRY-1230: Add filesystem tests to test Sentry with user data storage on S3. 
The tests can also be run on other types of fileSystems (ms azure, hdfs). Just 
ensure pass correct parameters.
For example,
   -Dsentry.e2etest.DFSType=S3DFS
   -Dsentry.e2etest.storage.uri=s3a://sentry-s3
   -Dfs.s3a.access.key=a
   -Dfs.s3a.secret.key=s
   export HIVE_CONF_DIR=/etc/hive/conf/hite-site.xml


Diffs (updated)
-

  pom.xml a82c66d9a730c76403600b12db7563522d6c12be 
  sentry-tests/sentry-tests-hive/pom.xml 
e36b5c04d48b3211abaecd36bf2673e6304bdc31 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
 001f5a49449112785cf129bfeda22c3bdf8cf3a6 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 0e4b3cab7685937e563b971274c66be43ba88fc1 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java
 32aabb41a1fef486ebed80571f0bd96e4c7c3c3c 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java
 06415cb828bfc6408bdc74d57c352ccfd55b7b43 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java
 67ba3386df6f52c1cd21d47eaec58a1a467bebcb 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
 7f650ceb01d6325c2dea794a207baaa5a4cb6e73 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/S3DFS.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestFSContants.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestHiveWarehouseOnExtFs.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestTableOnExtFS.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/AbstractHiveServer.java
 dda7dba662f7ba8fb9f705066c4bc680a7358f50 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/EmbeddedHiveServer.java
 52ba09e5e1006ff44a5073bce412ddee0774b24b 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServer.java
 175e84ceac8f24c1faa905d02daac1739e17f320 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java
 45f0ef28bc2393c16dd385cd8e37c133302b2993 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
 beae8e83bc9895f9c3aafd2eb9c6320b11ad0c1d 

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


Testing
---

---
 T E S T S
---
Running org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.419 sec - in 
org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

[INFO] 
[INFO] BUILD SUCCESS
[INFO] 
[INFO] Total time: 1:28.585s
[INFO] Finished at: Fri May 20 17:09:38 PDT 2016
[INFO] Final Memory: 60M/825M
[INFO] 


Thanks,

Anne Yu

FYI SENTRY-872 - Apache Sentry High Availability Active/Standby Redesig

[Anne Yu]

Colin posted "Apache Sentry High Availability Active/Standby Redesign". If
you feel interested, please take a look.

-- 
Thanks,
Anne

Re: Review Request 47937: SENTRY-1134: Add user defined udf test case.

[Anne Yu]

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




sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
 (line 118)
<https://reviews.apache.org/r/47937/#comment200362>

Belive this is not necessary; If you put any resource file into resources 
dir with the same root as src, shouldn't call this fashion of absolute path.



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
 (line 119)
<https://reviews.apache.org/r/47937/#comment200364>

Can you double check if sentry support add command? Seems we have 
blacklisted it for any user. Even if you grant permissions to user, he can't 
add resources. https://issues.apache.org/jira/browse/HIVE-5400



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
 (line 120)
<https://reviews.apache.org/r/47937/#comment200365>

BTW, can you add myudfs.java then let compiler compiles it, once create 
myudfs.jar will be put into target dir; your test can have access from there. 
So no need to check in myudfs.jar. Not sure if your compiled version will work 
on different systems or not (jdk7, 8 etc.)


- Anne Yu


On May 27, 2016, 1:33 a.m., Ke Jia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47937/
> ---
> 
> (Updated May 27, 2016, 1:33 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Dapeng Sun.
> 
> 
> Bugs: SENTRY-1134
> https://issues.apache.org/jira/browse/SENTRY-1134
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Add user defined UDF test, user create a jar then create udf from this jar.
> 
> 
> Diffs
> -
> 
>   .gitignore 80d5c03 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
>  ef7a86c 
>   sentry-tests/sentry-tests-hive/src/test/resources/myudfs.jar PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47937/diff/
> 
> 
> Testing
> ---
> 
> Testing
> 
> 
> Thanks,
> 
> Ke Jia
> 
>

Re: Review Request 47937: SENTRY-1134: Add user defined udf test case.

[Anne Yu]

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


Ship it!




LGTM. Please check in after addressing all the other reviewers' comments.

- Anne Yu


On June 1, 2016, 2:09 a.m., Ke Jia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47937/
> ---
> 
> (Updated June 1, 2016, 2:09 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Dapeng Sun.
> 
> 
> Bugs: SENTRY-1134
> https://issues.apache.org/jira/browse/SENTRY-1134
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Add user defined UDF test, user create a jar then create udf from this jar.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java
>  ef7a86c 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUDF.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47937/diff/
> 
> 
> Testing
> ---
> 
> Testing
> 
> 
> Thanks,
> 
> Ke Jia
> 
>

Re: Review Request 48111: SENTRY-950?add column level test cases for select ... group by, order by and where

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On June 1, 2016, 5:40 a.m., Ke Jia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48111/
> ---
> 
> (Updated June 1, 2016, 5:40 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Dapeng Sun.
> 
> 
> Bugs: SENTRY-950
> https://issues.apache.org/jira/browse/SENTRY-950
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> add column level test cases for select ... group by, order by and where.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithGrantOption.java
>  c67910a 
> 
> Diff: https://reviews.apache.org/r/48111/diff/
> 
> 
> Testing
> ---
> 
> Testing
> 
> 
> Thanks,
> 
> Ke Jia
> 
>

Re: postpone monthly meeting

[Anne Yu]

+1 for haohao's  suggestion.
On Jun 7, 2016 6:35 PM, "Hao Hao"  wrote:

> Next Thursday looks good to me. Happy holidays!
>
> Best,
> Hao
>
> On Tue, Jun 7, 2016 at 6:24 PM, Ma, Junjie  wrote:
>
> > Hi all,
> >
> > For the Chinese dragon boat  festival, I can't join the monthly meeting
> > this week, and suggest to postpone the monthly meeting to next week.
> >
> > best regards,
> > Colin Ma
> >
> >
>

Re: Review Request 48515: SENTRY-1320 : truncate table db_name.table_name fails

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On June 9, 2016, 9:39 p.m., Vihang Karajgaonkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48515/
> ---
> 
> (Updated June 9, 2016, 9:39 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1320
> https://issues.apache.org/jira/browse/SENTRY-1320
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Precondition check in PreAnalyze() expects only one child node in the ASTNode 
> for truncate query. But it is possible to have two children nodes when the 
> query is of the form truncate table db_name.table_name; Hive queries doing 
> this way to truncate the table fails will IllegalArgumentException during to 
> Precondition check violation.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
>  a13ab7918d8900f7dd83054f186b1fc0a04602c0 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScopePart2.java
>  3b5f6a6b593b281f7c6a34c5157d4dd929fa97bd 
> 
> Diff: https://reviews.apache.org/r/48515/diff/
> 
> 
> Testing
> ---
> 
> Ran the updated test TestPrivilegesAtTableScopePart2 successfully.
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>

Re: Review Request 48660: SENTRY-1319:Add metrics for isActive and isHA

[Anne Yu]

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java
 (line 123)
<https://reviews.apache.org/r/48660/#comment202508>

One important comment: wondering if there is going to be another cr to 
expose getIsActiveGauge and getIaHAGauge to any external app. External app 
doesn't have handle for sentry server.



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 (line 1728)
<https://reviews.apache.org/r/48660/#comment202509>

Can also add an e2e test case on a real cluster like, 
http://github.mtv.cloudera.com/CDH/sentry/blob/cdh5-1.5.1/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java.
 In that case, can sort of understanding how on unmanaged cluster, client gets 
to know the information.


- Anne Yu


On June 13, 2016, 6:52 p.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48660/
> ---
> 
> (Updated June 13, 2016, 6:52 p.m.)
> 
> 
> Review request for sentry, Colin McCabe and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1319:Add metrics for isActive and isHA
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java
>  c6d4d02fd77f4f9a31ea802c497fa23281bf7a75 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  578364933a3cdcf6c142b836360a83d322fe5c11 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  1606b6d97c6696328906100eb10464fdc2fa19e8 
> 
> Diff: https://reviews.apache.org/r/48660/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: [VOTE] Release Sentry version 1.7.0

[Anne Yu]

+1. Verified seems correct to me.

On Tue, Jun 7, 2016 at 11:58 PM, Hao Hao  wrote:

> Thanks a lot Colm for pointing out. I will add this to the release guide.
>
> Best,
> Hao
>
> On Tue, Jun 7, 2016 at 11:01 PM, Sravya Tirukkovalur 
> wrote:
>
> > Good catch Colm! I had Hao's public key locally as I fetched it when I
> > signed it earlier, so missed it. @Hao: We should also add this step to
> our
> > release guide. See here for details:
> > http://www.apache.org/dev/release-signing.html#keys-policy
> >
> > Regards,
> >
> > On Tue, Jun 7, 2016 at 9:53 AM, Colm O hEigeartaigh  >
> > wrote:
> >
> > > I guess "https://people.apache.org/keys/group/sentry.asc"; needs to be
> > > copied/merged to "
> https://dist.apache.org/repos/dist/release/sentry/KEYS
> > ".
> > > This should be done before the release is promoted. As I can now verify
> > the
> > > signature, it's +1 from me on the release.
> > >
> > > Colm.
> > >
> > >
> > >
> > > On Tue, Jun 7, 2016 at 5:45 PM, Colm O hEigeartaigh <
> cohei...@apache.org
> > >
> > > wrote:
> > >
> > >> Hi Stravya/Hao,
> > >>
> > >> It's strange, it looks like Firefox corrupts the ".sha" file when
> > >> downloading it. When I download it via "wget" it's fine. I guess we
> can
> > >> ignore this anyway.
> > >>
> > >> Everything looks good, however I can't verify the signature on the
> > >> release, as Hao's key is not included in:
> > >>
> > >> https://dist.apache.org/repos/dist/release/sentry/KEYS
> > >>
> > >> The only keys in there are:
> > >>
> > >> gpg: key 22E26224: public key "Shreepadma Venugopalan (CODE SIGNING
> > KEY) <
> > >> shreepa...@apache.org>" imported
> > >> gpg: key 7D66174C: public key "Tuong Truong (CODE SIGNING KEY) <
> > >> tuong.tru...@gmail.com>" imported
> > >> gpg: key E2DE1E32: public key "gqshen (CODE SIGNING KEY) <
> > >> gqs...@apache.org>" imported
> > >> gpg: key AED7ED79: public key "Dapeng Sun (CODE SIGNING KEY) <
> > >> s...@apache.org>" imported
> > >>
> > >> Colm.
> > >>
> > >> On Fri, Jun 3, 2016 at 9:43 PM, Sravya Tirukkovalur <
> > sra...@cloudera.com>
> > >> wrote:
> > >>
> > >>> I checked as well and sha seems fine to me too. @Colm what exactly do
> > >>> you see ?
> > >>>
> > >>> +1 from me.
> > >>>
> > >>> Verified the following.
> > >>>
> > >>>- Made sure RCs are hosted @
> > >>>https://dist.apache.org/repos/dist/dev/sentry
> > >>>- Is in format apache-$project-$version.tar.gz
> > >>>- Verified Signatures and hashes.
> > >>>- git tag matches the released bits (diff -rf)
> > >>>- Can compile successfully from source
> > >>>- Verified NOTICE has correct year
> > >>>- All files have correct headers (Rat check is clean)
> > >>>- No jar files in the release
> > >>>
> > >>>
> > >>> Regards,
> > >>>
> > >>> On Fri, Jun 3, 2016 at 11:59 AM, Hao Hao 
> wrote:
> > >>>
> >  Hi Colm,
> > 
> >  Thanks for checking! But not sure how did you verify it? I
> downloaded
> >  the
> >  sha file and it seems look good to me:
> > 
> >  cat apache-sentry-1.7.0-src.tar.gz.sha
> >  81c23908bc35e79a1a7c7e031cb904ee187f12bb
> > apache-sentry-1.7.0-src.tar.gz
> > 
> >  Does anyone else give it a try? Thanks a lot!
> > 
> >  Best,
> >  Hao
> > 
> >  On Thu, Jun 2, 2016 at 7:57 AM, Colm O hEigeartaigh <
> >  cohei...@apache.org>
> >  wrote:
> > 
> >  > I think the ".sha" file is corrupted, it appears to contain binary
> >  data.
> >  >
> >  > Colm.
> >  >
> >  > On Thu, Jun 2, 2016 at 7:56 AM, Hao Hao 
> > wrote:
> >  >
> >  > > Hi all,
> >  > >
> >  > > This is the release of Apache Sentry, version 1.7.0. The list of
> >  fixed
> >  > > issues, added features and improvements can be found here:
> >  > > https://s.apache.org/FTD1
> >  > >
> >  > > Source files:
> >  https://dist.apache.org/repos/dist/dev/sentry/1.7.0-rc0/
> >  > >
> >  > > Tag to be voted on (rc0):
> >  > >
> >  > >
> >  >
> > 
> >
> https://git-wip-us.apache.org/repos/asf?p=sentry.git;a=commit;h=735543e71d478f5bcd6be2b991ed26fc95abbb1b
> >  > > Sentry's KEYS containing the PGP key we used to sign the
> release:
> >  > > https://people.apache.org/keys/group/sentry.asc
> >  > >
> >  > > Note that this is a source only release and we are voting on the
> >  > > source: tag=release-1.7.0,
> >  SHA=735543e71d478f5bcd6be2b991ed26fc95abbb1b
> >  > >
> >  > >
> >  > > Vote will be open for 72 hours.
> >  > >
> >  > > [ ] +1 approve
> >  > > [ ] +0 no opinion
> >  > > [ ] -1 disapprove (and reason why)
> >  > >
> >  > >
> >  > > Thanks,
> >  > > Hao
> >  > >
> >  >
> >  >
> >  >
> >  > --
> >  > Colm O hEigeartaigh
> >  >
> >  > Talend Community Coder
> >  > http://coders.talend.com
> >  >
> > 
> > >>>
> > >>>
> > >>>
> > >>> --
> > >>> Sravya Tirukkovalur
> > >>>
> > >>
> > >>
> > >>
> > >> --
> 

Re: fixVersion for Resolved JIRA

[Anne Yu]

Yeah it will help analyze jira and categorize issues later on.
On Jun 16, 2016 5:02 PM, "Hao Hao"  wrote:

> Hi all,
>
> While doing 1.7.0 release, I noticed some resolved Jiras do not have
> fixVersion. I feel it would be good to require a fixVersion once the Jira
> is resolved. So I filed a Infra Jira request:
> https://issues.apache.org/jira/browse/INFRA-12115. Any opinions? :)Thanks!
>
> Best,
> Hao
>

Re: Review Request 48770: SENTRY-1325: Store HMSPaths in Sentry DB to allow fast failover

[Anne Yu]

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



Thanks Haohao. Left some minor comments.


sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
 (line 122)
<https://reviews.apache.org/r/48770/#comment204065>

Not sure why here it's final. If means it is not mutable, maybe use a 
different datastructure, such as, ImmutableMap (Google).



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
 (line 342)
<https://reviews.apache.org/r/48770/#comment204063>

Whitespace.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
 (line 25)
<https://reviews.apache.org/r/48770/#comment204068>

Transactional database backed table for 



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
 (line 95)
<https://reviews.apache.org/r/48770/#comment204069>

Just want to confirm if want to double check path content for equality?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
 (line 248)
<https://reviews.apache.org/r/48770/#comment204070>

name="value" maybe keep it consistent by using whitespace or no-whitespace.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 (line 64)
<https://reviews.apache.org/r/48770/#comment204071>

import alhpabetic asending order?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 (line 2002)
<https://reviews.apache.org/r/48770/#comment204073>

Just want to confirm there is no two authzToPaths objects with the same 
getAuthzObjName case; if so, might check existence then update value.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 (line 2008)
<https://reviews.apache.org/r/48770/#comment204074>

Didn't find where to set rollbackTransaction = false?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
 (line 2084)
<https://reviews.apache.org/r/48770/#comment204076>

Just curious, in this case, will the below two cases are the same?

hdfs://nameservice/user/hive/warehouse/db1.db/table1 and db1.db/table1?
    
whether the same or not, could add a test case to check?


- Anne Yu


On June 17, 2016, 8:32 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48770/
> ---
> 
> (Updated June 17, 2016, 8:32 p.m.)
> 
> 
> Review request for sentry, Colin McCabe and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1325: Store HMSPaths in Sentry DB to allow fast failover
> 
> Change-Id: Icfb86d86b8cdbbf786c48780cc3ee8ec8a2b2698
> 
> Added schema for storing HMSPaths: MSentryPathsUpdate. It has hiveObj, paths, 
> createTime.
> Added datastore APIs: retrieveFullPathsImage, and createSentryPathsUpdate.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  6d2ab23c3f65af5430ea02563e13c4c4c49aa1c6 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java
>  e759ff1229353570c5fc1a8547f47127e2dabff1 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
>  b3b949400c2a6dd5797a678da8966617b0598cca 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  b7ef0e94fac911e97e4ae9ba1cde49edbe1eb62b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  bc7fe12b732e523ad6f50fd1daf5078cc37614f1 
> 
> Diff: https://reviews.apache.org/r/48770/diff/
> 
> 
> Testing
> ---
> 
> Added a test case in TestSentryStore#testPathsUpdate.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

RE: fixVersion for Resolved JIRA

[Anne Yu]

Yeah I think it's fix version whenever one is resolving the jira after
patch committed.

Thanks,
Anne
On Jun 22, 2016 1:03 AM, "Sun, Dapeng"  wrote:

> Hi Anne and Hao
>
> IMO, we only know which version would be the fixVersion after the patch
> committed. Do you mean affectVersion?
>
> Regards
> Dapeng
>
> -Original Message-
> From: Anne Yu [mailto:ann...@cloudera.com]
> Sent: Friday, June 17, 2016 8:54 AM
> To: sentry
> Subject: Re: fixVersion for Resolved JIRA
>
> Yeah it will help analyze jira and categorize issues later on.
> On Jun 16, 2016 5:02 PM, "Hao Hao"  wrote:
>
> > Hi all,
> >
> > While doing 1.7.0 release, I noticed some resolved Jiras do not have
> > fixVersion. I feel it would be good to require a fixVersion once the
> > Jira is resolved. So I filed a Infra Jira request:
> > https://issues.apache.org/jira/browse/INFRA-12115. Any opinions?
> :)Thanks!
> >
> > Best,
> > Hao
> >
>

Re: sentry git commit: SENTRY-1348: Move HA related class from sentry-provider-db to sentry-service-common(Colin Ma, reviewed by Dapeng Sun)

[Anne Yu]

Hi Colin and Team,

We are implementing Sentry-ha redesign in the branch *redesign-sentry-ha*,
do you think this commit need also go into that branch. From now on, we
might want to check in ha fixes in both trunk and branch.

Thanks,
Anne
On Jun 21, 2016 10:27 PM,  wrote:

> Repository: sentry
> Updated Branches:
>   refs/heads/SENTRY-1205 aef769858 -> 4767ec38e
>
>
> SENTRY-1348: Move HA related class from sentry-provider-db to
> sentry-service-common(Colin Ma, reviewed by Dapeng Sun)
>
>
> Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
> Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/4767ec38
> Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/4767ec38
> Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/4767ec38
>
> Branch: refs/heads/SENTRY-1205
> Commit: 4767ec38ef2a5d967d37539c64c787c986afe5cb
> Parents: aef7698
> Author: Colin Ma 
> Authored: Wed Jun 22 14:11:57 2016 +0800
> Committer: Colin Ma 
> Committed: Wed Jun 22 14:11:57 2016 +0800
>
> --
>  sentry-provider/sentry-provider-db/pom.xml  |  17 --
>  .../persistent/FixedJsonInstanceSerializer.java | 163 
>  .../db/service/persistent/HAContext.java| 262 ---
>  .../service/thrift/JaasConfiguration.java   | 133 --
>  sentry-service/sentry-service-common/pom.xml|  17 ++
>  .../persistent/FixedJsonInstanceSerializer.java | 163 
>  .../db/service/persistent/HAContext.java| 262 +++
>  .../service/thrift/JaasConfiguration.java   | 133 ++
>  8 files changed, 575 insertions(+), 575 deletions(-)
> --
>
>
>
> http://git-wip-us.apache.org/repos/asf/sentry/blob/4767ec38/sentry-provider/sentry-provider-db/pom.xml
> --
> diff --git a/sentry-provider/sentry-provider-db/pom.xml
> b/sentry-provider/sentry-provider-db/pom.xml
> index f3029fa..3d76198 100644
> --- a/sentry-provider/sentry-provider-db/pom.xml
> +++ b/sentry-provider/sentry-provider-db/pom.xml
> @@ -38,11 +38,6 @@ limitations under the License.
>  
>  
>org.apache.hadoop
> -  hadoop-common
> -  provided
> -
> -
> -  org.apache.hadoop
>hadoop-mapreduce-client-jobclient
>  
>  
> @@ -166,18 +161,6 @@ limitations under the License.
>test
>  
>  
> -  org.apache.curator
> -  curator-recipes
> -
> -
> -  org.apache.curator
> -  curator-x-discovery
> -
> -
> -  org.apache.curator
> -  curator-test
> -
> -
>org.apache.commons
>commons-pool2
>  
>
>
> http://git-wip-us.apache.org/repos/asf/sentry/blob/4767ec38/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java
> --
> diff --git
> a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java
> b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java
> deleted file mode 100644
> index 476bf6a..000
> ---
> a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java
> +++ /dev/null
> @@ -1,163 +0,0 @@
> -/**
> - * Licensed to the Apache Software Foundation (ASF) under one
> - * or more contributor license agreements.  See the NOTICE file
> - * distributed with this work for additional information
> - * regarding copyright ownership.  The ASF licenses this file
> - * to you under the Apache License, Version 2.0 (the
> - * "License"); you may not use this file except in compliance
> - * with the License.  You may obtain a copy of the License at
> - *
> - *  http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -
> -package org.apache.sentry.provider.db.service.persistent;
> -
> -import java.io.ByteArrayInputStream;
> -import java.io.ByteArrayOutputStream;
> -import java.io.IOException;
> -
> -import org.codehaus.jackson.JsonNode;
> -import org.codehaus.jackson.JsonParseException;
> -import org.codehaus.jackson.map.DeserializationConfig;
> -import org.codehaus.jackson.map.JsonMappingException;
> -import org.codehaus.jackson.map.ObjectMapper;
> -
> -import com.google.common.base.Preconditions;
> -import org.apache.curator.x.discovery.ServiceInstance;
> -import org.apache.curator.x.discover

Re: sentry git commit: SENTRY-1348: Move HA related class from sentry-provider-db to sentry-service-common(Colin Ma, reviewed by Dapeng Sun)

[Anne Yu]

I am thinking of the other way, when folks check sentry ha related stuffs
into master could merge into redesign-sentry-ha branch. There are also new
tests we might want to have to validate the feature branch.

Best,
Anne

On Wed, Jun 22, 2016 at 11:18 AM, Colin McCabe  wrote:

> Let's try to focus on getting our branch done, and then worry about
> merging back to master.
>
> People move stuff around all the time, and following the latest churn
> takes time.
>
> Colin
>
> On Wed, Jun 22, 2016 at 9:45 AM, Anne Yu  wrote:
>
>> Hi Colin and Team,
>>
>> We are implementing Sentry-ha redesign in the branch
>> *redesign-sentry-ha*,  do you think this commit need also go into that
>> branch. From now on, we might want to check in ha fixes in both trunk and
>> branch.
>>
>> Thanks,
>> Anne
>> On Jun 21, 2016 10:27 PM,  wrote:
>>
>>> Repository: sentry
>>> Updated Branches:
>>>   refs/heads/SENTRY-1205 aef769858 -> 4767ec38e
>>>
>>>
>>> SENTRY-1348: Move HA related class from sentry-provider-db to
>>> sentry-service-common(Colin Ma, reviewed by Dapeng Sun)
>>>
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/4767ec38
>>> Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/4767ec38
>>> Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/4767ec38
>>>
>>> Branch: refs/heads/SENTRY-1205
>>> Commit: 4767ec38ef2a5d967d37539c64c787c986afe5cb
>>> Parents: aef7698
>>> Author: Colin Ma 
>>> Authored: Wed Jun 22 14:11:57 2016 +0800
>>> Committer: Colin Ma 
>>> Committed: Wed Jun 22 14:11:57 2016 +0800
>>>
>>> --
>>>  sentry-provider/sentry-provider-db/pom.xml  |  17 --
>>>  .../persistent/FixedJsonInstanceSerializer.java | 163 
>>>  .../db/service/persistent/HAContext.java| 262
>>> ---
>>>  .../service/thrift/JaasConfiguration.java   | 133 --
>>>  sentry-service/sentry-service-common/pom.xml|  17 ++
>>>  .../persistent/FixedJsonInstanceSerializer.java | 163 
>>>  .../db/service/persistent/HAContext.java| 262
>>> +++
>>>  .../service/thrift/JaasConfiguration.java   | 133 ++
>>>  8 files changed, 575 insertions(+), 575 deletions(-)
>>> --
>>>
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/sentry/blob/4767ec38/sentry-provider/sentry-provider-db/pom.xml
>>> --
>>> diff --git a/sentry-provider/sentry-provider-db/pom.xml
>>> b/sentry-provider/sentry-provider-db/pom.xml
>>> index f3029fa..3d76198 100644
>>> --- a/sentry-provider/sentry-provider-db/pom.xml
>>> +++ b/sentry-provider/sentry-provider-db/pom.xml
>>> @@ -38,11 +38,6 @@ limitations under the License.
>>>  
>>>  
>>>org.apache.hadoop
>>> -  hadoop-common
>>> -  provided
>>> -
>>> -
>>> -  org.apache.hadoop
>>>hadoop-mapreduce-client-jobclient
>>>  
>>>  
>>> @@ -166,18 +161,6 @@ limitations under the License.
>>>test
>>>  
>>>  
>>> -  org.apache.curator
>>> -  curator-recipes
>>> -
>>> -
>>> -  org.apache.curator
>>> -  curator-x-discovery
>>> -
>>> -
>>> -  org.apache.curator
>>> -  curator-test
>>> -
>>> -
>>>org.apache.commons
>>>commons-pool2
>>>  
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/sentry/blob/4767ec38/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java
>>> --
>>> diff --git
>>> a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java
>>> b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FixedJsonInstanceSerializer.java
>>> deleted file mode 100644
>>> index 476bf6a..000
>>> ---
&g

Re: meeting minutes for monthly call (2016/06/16)

[Anne Yu]

Regarding 5, Record Service has an infra implemented to pull sentry/hive
binaries, configure environments, and start services. (Sample code
).
If you have any other questions can also ask LiLi .

Best,
Anne

On Wed, Jun 22, 2016 at 1:11 AM, Sun, Dapeng  wrote:

> Thank Colin for the minutes!
>
> Regards
> Dapeng
>
> -Original Message-
> From: Ma, Junjie [mailto:junjie...@intel.com]
> Sent: Friday, June 17, 2016 10:02 AM
> To: dev@sentry.apache.org
> Subject: meeting minutes for monthly call (2016/06/16)
>
> Hi all,
>
> The following is today's meeting minutes:
>
> 1.  For the Sentry HA redesign, Dapeng will investigate the related
> JIRAs, eg, SENTRY-1323
>
> 2.  Colin will add the performance test case and take the SENTRY-1295.
>
> 3.  Sravya will help to review SENTRY-1209.
>
> 4.  The refacor for split sentry-server and sentry-client is still in
> development.
>
> 5.  Anne will share the script for setting up the cluster to do the
> e2e test.
>
> 6.  The "owner concept" which exist in Hive should be added to Sentry.
>
> best regards,
> Colin Ma
>
>


-- 
Anne

Re: Review Request 48770: SENTRY-1325: Store HMSPaths in Sentry DB to allow fast failover

[Anne Yu]

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


Ship it!




LGTM.

- Anne Yu


On June 23, 2016, 1:30 a.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48770/
> ---
> 
> (Updated June 23, 2016, 1:30 a.m.)
> 
> 
> Review request for sentry, Colin McCabe and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1325: Store HMSPaths in Sentry DB to allow fast failover
> 
> Change-Id: Icfb86d86b8cdbbf786c48780cc3ee8ec8a2b2698
> 
> Added schema for storing HMSPaths: MSentryPathsUpdate. It has hiveObj, paths, 
> createTime.
> Added datastore APIs: retrieveFullPathsImage, and createSentryPathsUpdate.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
>  6d2ab23c3f65af5430ea02563e13c4c4c49aa1c6 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java
>  e759ff1229353570c5fc1a8547f47127e2dabff1 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
>  b3b949400c2a6dd5797a678da8966617b0598cca 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  b7ef0e94fac911e97e4ae9ba1cde49edbe1eb62b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  bc7fe12b732e523ad6f50fd1daf5078cc37614f1 
> 
> Diff: https://reviews.apache.org/r/48770/diff/
> 
> 
> Testing
> ---
> 
> Added a test case in TestSentryStore#testPathsUpdate.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 49290: SENTRY-1368: Test Alter table rename for managed/unmanaged, with/without partitions

[Anne Yu]

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




sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 (line 1802)
<https://reviews.apache.org/r/49290/#comment205160>

White space here.


A general comment. We have had testing running too long and vm gets killed 
issue. So for each test class, ideally it is a small and focused test suite. If 
it grows large, can put into a seperate test class. So ensure each test class 
runs no more than (10m).

- Anne Yu


On June 28, 2016, 12:27 a.m., Sravya Tirukkovalur wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49290/
> ---
> 
> (Updated June 28, 2016, 12:27 a.m.)
> 
> 
> Review request for sentry, Anne Yu, Hao Hao, Kapil Rastogi, Lenni Kuff, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Adding test cases for rename table. Details are in the test class itself.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b1d507f591323e08e0d0a0384a4525ad8b66af0c 
> 
> Diff: https://reviews.apache.org/r/49290/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sravya Tirukkovalur
> 
>

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)
<https://reviews.apache.org/r/49491/#comment205779>

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
> 
>

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
> > <https://reviews.apache.org/r/49491/diff/1/?file=1434660#file1434660line633>
> >
> > 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 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)
<https://reviews.apache.org/r/49526/#comment205785>

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)
<https://reviews.apache.org/r/49526/#comment205787>

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
> 
>

Re: Review Request 49526: SENTRY-1365

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On July 5, 2016, 11:54 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49526/
> ---
> 
> (Updated July 5, 2016, 11:54 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-db2-1.8.0.sql 
> 6d08b5c0bf29a2c33f0baf5701c3efaf4b5d29d3 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.8.0.sql 
> d522026e18e6c2532487e9d4a988e408e6736008 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.8.0.sql 
> d27d7b93cd3fb50d54c14c4eddee1ce59c785b13 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.8.0.sql 
> ced5c31a6a1d12dec88c39132797cf89dfa36d0f 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.8.0.sql
>  6d563324bdf631790edce512502394b8fa81bd1f 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-1.8.0.sql
>  927f302f789913a787b177b89af8b90a23c013ca 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-derby-1.7.0-to-1.8.0.sql
>  fbe2dc8e285c2459c4563cda7156ce83bdf66a0a 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-mysql-1.7.0-to-1.8.0.sql
>  f0df187ab3299f0c87bb26eb32bee0001f45fc5a 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-oracle-1.7.0-to-1.8.0.sql
>  f1666be9244ada88c59107f564d7c7662299f851 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-postgres-1.7.0-to-1.8.0.sql
>  b39292d621ad68ff62f2585372a58c28e0eb8d37 
> 
> Diff: https://reviews.apache.org/r/49526/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

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
> > <https://reviews.apache.org/r/49491/diff/1/?file=1434660#file1434660line633>
> >
> > 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 wrote:
> 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?
> 
> Ke Jia wrote:
> Hi Anne Yu,  Whether the "dbprovider profile" is a profile of maven? Now, 
> I updated the patch, but I don't know how run it in dbprovider profile on a 
> real cluster.

Oh, as long as the test is included into "dbprovider profile" is able to run 
it. What I meant is ensure it's included into that profile.


- 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/#review141639
---


Ship it!




- Anne Yu


On July 11, 2016, 1:26 a.m., Ke Jia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49491/
> ---
> 
> (Updated July 11, 2016, 1:26 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/dbprovider/TestColumnEndToEnd.java
>  22d4cf8 
> 
> Diff: https://reviews.apache.org/r/49491/diff/
> 
> 
> Testing
> ---
> 
> test done
> 
> 
> Thanks,
> 
> Ke Jia
> 
>

Re: Review Request 49526: SENTRY-1365

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On July 11, 2016, 7:36 a.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49526/
> ---
> 
> (Updated July 11, 2016, 7:36 a.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-db2-1.8.0.sql 
> 6d08b5c0bf29a2c33f0baf5701c3efaf4b5d29d3 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.8.0.sql 
> d522026e18e6c2532487e9d4a988e408e6736008 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.8.0.sql 
> d27d7b93cd3fb50d54c14c4eddee1ce59c785b13 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.8.0.sql 
> ced5c31a6a1d12dec88c39132797cf89dfa36d0f 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.8.0.sql
>  6d563324bdf631790edce512502394b8fa81bd1f 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-1.8.0.sql
>  927f302f789913a787b177b89af8b90a23c013ca 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-derby-1.7.0-to-1.8.0.sql
>  fbe2dc8e285c2459c4563cda7156ce83bdf66a0a 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-mysql-1.7.0-to-1.8.0.sql
>  f0df187ab3299f0c87bb26eb32bee0001f45fc5a 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-oracle-1.7.0-to-1.8.0.sql
>  f1666be9244ada88c59107f564d7c7662299f851 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-postgres-1.7.0-to-1.8.0.sql
>  b39292d621ad68ff62f2585372a58c28e0eb8d37 
> 
> Diff: https://reviews.apache.org/r/49526/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 50264: SENTRY-1331: Add a kerberos end to end test case to access isActive and isHa metrics.

[Anne Yu]

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




sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
 (line 26)
<https://reviews.apache.org/r/50264/#comment208684>

Thank you for your nice work on adding more tests. 

For good practice, try not to import .*;



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
 (line 36)
<https://reviews.apache.org/r/50264/#comment208688>

kerberos = true instead ? or you meant to have two test methods, one for 
non-kerberos and the other for kerberos.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
 (line 63)
<https://reviews.apache.org/r/50264/#comment208685>

Do we always use http not https for all metrics?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
 (line 80)
<https://reviews.apache.org/r/50264/#comment208686>

Not sure what is this empty method for?



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
 (line 142)
<https://reviews.apache.org/r/50264/#comment208687>

To me isXXX natuarally returns TRUE or FALSE. Seems 
mp.isActive().getBooleanValue() also validate boolean results. Not sure why 
here return JsonNode obj.


- Anne Yu


On July 20, 2016, 9:24 p.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50264/
> ---
> 
> (Updated July 20, 2016, 9:24 p.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1331: Add a kerberos end to end test case to access isActive and isHa 
> metrics.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
>  3fff450c23d1ecd624d6fe406d5a7920b94fadd0 
> 
> Diff: https://reviews.apache.org/r/50264/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50264: SENTRY-1331: Add a kerberos end to end test case to access isActive and isHa metrics.

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On July 22, 2016, 8:36 p.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50264/
> ---
> 
> (Updated July 22, 2016, 8:36 p.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1331: Add a kerberos end to end test case to access isActive and isHa 
> metrics.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
>  3fff450c23d1ecd624d6fe406d5a7920b94fadd0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryWebMetricParser.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50264/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

[Anne Yu]

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




sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
 (line 42)
<https://reviews.apache.org/r/50404/#comment209409>

It would be better to test like the below:

1. stop the active server;
2. assert active server is stopped correctly; give test a time out, in case 
in a freezing mode;
3. ensure passive server to become active; meaning the new active server is 
the previous passive one;


- Anne Yu


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

[Anne Yu]

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




sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
 (line 47)
<https://reviews.apache.org/r/50404/#comment209413>

Will it be possible to have more than 2 servers?


- Anne Yu


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

[Anne Yu]

> On July 26, 2016, 6:39 p.m., Anne Yu wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java,
> >  line 42
> > <https://reviews.apache.org/r/50404/diff/2/?file=1452126#file1452126line42>
> >
> > It would be better to test like the below:
> > 
> > 1. stop the active server;
> > 2. assert active server is stopped correctly; give test a time out, in 
> > case in a freezing mode;
> > 3. ensure passive server to become active; meaning the new active 
> > server is the previous passive one;
> 
> Rahul Sharma wrote:
> So this is just for a single service unit case, to verify that the server 
> is down. I have added a similar case to what you suggest in the 
> TestFailover.java

ic, sounds good. Thanks for clarification.


- Anne


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


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> -------
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

[Anne Yu]

> On July 26, 2016, 7:16 p.m., Anne Yu wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java,
> >  line 47
> > <https://reviews.apache.org/r/50404/diff/2/?file=1452127#file1452127line47>
> >
> > Will it be possible to have more than 2 servers?
> 
> Rahul Sharma wrote:
> I just used the InternalSrv, which current supports 2 for HA scenario.

oh, I am changing that to >= 2 case; I meant by design, seems ha supports >= 2 
case. If so test needs to accommodate it; if not, you can ignore my comment.


- Anne


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


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

[Anne Yu]

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




sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
 (line 34)
<https://reviews.apache.org/r/50404/#comment209436>

Seems this assignment doesn't enable sentry ha. It only validate one system 
property,

if ("true".equalsIgnoreCase(System.getProperty(ENABLE_SENTRY_HA, "false"))) 
{
  enableSentryHA = true;
}


- Anne Yu


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

[Anne Yu]

> On July 27, 2016, 9:37 p.m., Rahul Sharma wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java,
> >  line 47
> > <https://reviews.apache.org/r/50404/diff/2/?file=1452127#file1452127line47>
> >
> > Are you making that configurable anywhere, like number of servers to be 
> > used? . If yes, then if you commit that code, I can change this case.

Yeah, I am going to make it configurable passing from maven; default value will 
still be 2. In this case, you might want to make it a loop to look for active 
server id.


- Anne


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


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

[Anne Yu]

> On July 27, 2016, 9:37 p.m., Rahul Sharma wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java,
> >  line 47
> > <https://reviews.apache.org/r/50404/diff/2/?file=1452127#file1452127line47>
> >
> > Are you making that configurable anywhere, like number of servers to be 
> > used? . If yes, then if you commit that code, I can change this case.
> 
> Anne Yu wrote:
> Yeah, I am going to make it configurable passing from maven; default 
> value will still be 2. In this case, you might want to make it a loop to look 
> for active server id.
> 
> Rahul Sharma wrote:
> I think it should be configurable through the tests also ? someting like 
> numofServer(defaults to 1) in AbstractTestWithStaticConfiguration

Following up jira: https://issues.apache.org/jira/browse/SENTRY-1432.


- Anne


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


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> -------
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Review Request 50611: Sentry HA Test: programmatic failover, validate privileges before and afterwards.

[Anne Yu]

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

Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Bugs: SENTRY-1423
https://issues.apache.org/jira/browse/SENTRY-1423


Repository: sentry


Description
---

Sentry HA Test: test programmatic failover, validate privileges before and 
afterwards. validate expected exception is thrown.


Diffs
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestProgrammaticFailover.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 7dc3d0f30583b9edd16015218b199e298d192e8c 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
 054b1935975368db058791102f3892f027e31636 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
 dac11517f36a743dff8730465fbf2397ebf16c88 

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


Testing
---


Thanks,

Anne Yu

Re: Review Request 50474: SENTRY-1404: Use the new INodeAttributesProvider API in sentry-hdfs

[Anne Yu]

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



Thanks Haohao. Left some minor comments here.


sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationConstants.java
 (line 32)
<https://reviews.apache.org/r/50474/#comment210029>

Minor issues: white spaces.



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
 (line 75)
<https://reviews.apache.org/r/50474/#comment210035>

Just want to enforce defensive coding, pathElems != null && 
pathElems.length > 1 ...



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
 (line 156)
<https://reviews.apache.org/r/50474/#comment210143>

Will it be possible prefixPath contains the same items with different order 
from pathElements?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
 (line 198)
<https://reviews.apache.org/r/50474/#comment210144>

Better add some comments for each condition branch to state which case it 
is trying to handle. Roughtly seems could be 6 cases+/-. It will help maintain 
codes and support escalations.



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
 (line 332)
<https://reviews.apache.org/r/50474/#comment210148>

pathElements != null && ...



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 (line 1967)
<https://reviews.apache.org/r/50474/#comment210152>

Not sure if the other group/user's rwx should be set by sentry privileges 
or just hard set by test?


- Anne Yu


On July 26, 2016, 11:41 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50474/
> ---
> 
> (Updated July 26, 2016, 11:41 p.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Use the new INodeAttributesProvider API in sentry-hdfs
> 
> 
> Diffs
> -
> 
>   pom.xml a4f2bcc14855a2e5f712f9c56fbdb1e137edfa93 
>   sentry-binding/sentry-binding-solr/pom.xml 
> 8b94c87118841c2ce775c3e79ff6bb43ba5185fd 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/hadoop/hdfs/server/namenode/AuthorizationProvider.java
>  383d64d88c74c93cd79ccb189301b9c1a6d919e4 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationConstants.java
>  883680182fd01f4b0b1a7fb5d8eb4d7f23f5851e 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
>  f639f5fbacc85b2adb757087c2eba66b092743a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/MockSentryAuthorizationProvider.java
>  2085b525db0b7a9fda77a4be62026da3bf74b7ac 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
>  5da0dc2fb4a47098bd5f9b36dbacb0296162f95b 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  60085b20774fb28c978f60b97bbef0ecbadb8d8f 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  25d13d62a1a16beba16ae46fbd28f2490fca10bc 
> 
> Diff: https://reviews.apache.org/r/50474/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 50611: Sentry HA Test: programmatic failover, validate privileges before and afterwards.

[Anne Yu]

> On July 29, 2016, 6:26 p.m., Rahul Sharma wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestProgrammaticFailover.java,
> >  lines 117-120
> > <https://reviews.apache.org/r/50611/diff/1/?file=1457990#file1457990line117>
> >
> > how is the boolean return value from failover() useful?

Yeah, will validate this boolean value before move forward to validation.


> On July 29, 2016, 6:26 p.m., Rahul Sharma wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java,
> >  line 36
> > <https://reviews.apache.org/r/50611/diff/1/?file=1457991#file1457991line36>
> >
> > Logger for this class?

Good catch, will modify this to the current class.


- Anne


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


On July 29, 2016, 5:27 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50611/
> ---
> 
> (Updated July 29, 2016, 5:27 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1423
> https://issues.apache.org/jira/browse/SENTRY-1423
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Sentry HA Test: test programmatic failover, validate privileges before and 
> afterwards. validate expected exception is thrown.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestProgrammaticFailover.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  7dc3d0f30583b9edd16015218b199e298d192e8c 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
>  054b1935975368db058791102f3892f027e31636 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
>  dac11517f36a743dff8730465fbf2397ebf16c88 
> 
> Diff: https://reviews.apache.org/r/50611/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 50611: Sentry HA Test: programmatic failover, validate privileges before and afterwards.

[Anne Yu]

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

(Updated July 29, 2016, 8:03 p.m.)


Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Changes
---

1. addressed Rahul's comments;
2. enable sentry client pool for testing;
3. ensure can parse external test configurations and internally specified test 
values.


Bugs: SENTRY-1423
https://issues.apache.org/jira/browse/SENTRY-1423


Repository: sentry


Description
---

Sentry HA Test: test programmatic failover, validate privileges before and 
afterwards. validate expected exception is thrown.


Diffs (updated)
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestProgrammaticFailover.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 7dc3d0f30583b9edd16015218b199e298d192e8c 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
 054b1935975368db058791102f3892f027e31636 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
 dac11517f36a743dff8730465fbf2397ebf16c88 

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


Testing
---


Thanks,

Anne Yu

Re: Review Request 50474: SENTRY-1404: Use the new INodeAttributesProvider API in sentry-hdfs

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On July 29, 2016, 7:41 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50474/
> ---
> 
> (Updated July 29, 2016, 7:41 p.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Use the new INodeAttributesProvider API in sentry-hdfs
> 
> 
> Diffs
> -
> 
>   pom.xml a4f2bcc14855a2e5f712f9c56fbdb1e137edfa93 
>   sentry-binding/sentry-binding-solr/pom.xml 
> 8b94c87118841c2ce775c3e79ff6bb43ba5185fd 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/hadoop/hdfs/server/namenode/AuthorizationProvider.java
>  383d64d88c74c93cd79ccb189301b9c1a6d919e4 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationConstants.java
>  883680182fd01f4b0b1a7fb5d8eb4d7f23f5851e 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
>  f639f5fbacc85b2adb757087c2eba66b092743a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/MockSentryAuthorizationProvider.java
>  2085b525db0b7a9fda77a4be62026da3bf74b7ac 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
>  5da0dc2fb4a47098bd5f9b36dbacb0296162f95b 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  60085b20774fb28c978f60b97bbef0ecbadb8d8f 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  25d13d62a1a16beba16ae46fbd28f2490fca10bc 
> 
> Diff: https://reviews.apache.org/r/50474/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 50474: SENTRY-1404: Use the new INodeAttributesProvider API in sentry-hdfs

[Anne Yu]

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




sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationConstants.java
 (line 55)
<https://reviews.apache.org/r/50474/#comment210187>

Missing one white space.


- Anne Yu


On July 29, 2016, 7:41 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50474/
> ---
> 
> (Updated July 29, 2016, 7:41 p.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Use the new INodeAttributesProvider API in sentry-hdfs
> 
> 
> Diffs
> -
> 
>   pom.xml a4f2bcc14855a2e5f712f9c56fbdb1e137edfa93 
>   sentry-binding/sentry-binding-solr/pom.xml 
> 8b94c87118841c2ce775c3e79ff6bb43ba5185fd 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/hadoop/hdfs/server/namenode/AuthorizationProvider.java
>  383d64d88c74c93cd79ccb189301b9c1a6d919e4 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationConstants.java
>  883680182fd01f4b0b1a7fb5d8eb4d7f23f5851e 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
>  f639f5fbacc85b2adb757087c2eba66b092743a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/MockSentryAuthorizationProvider.java
>  2085b525db0b7a9fda77a4be62026da3bf74b7ac 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
>  5da0dc2fb4a47098bd5f9b36dbacb0296162f95b 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  60085b20774fb28c978f60b97bbef0ecbadb8d8f 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  25d13d62a1a16beba16ae46fbd28f2490fca10bc 
> 
> Diff: https://reviews.apache.org/r/50474/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 50474: SENTRY-1404: Use the new INodeAttributesProvider API in sentry-hdfs

[Anne Yu]

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


Ship it!




Ship It!

- Anne Yu


On Aug. 2, 2016, 8:36 p.m., Hao Hao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50474/
> ---
> 
> (Updated Aug. 2, 2016, 8:36 p.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Use the new INodeAttributesProvider API in sentry-hdfs
> 
> 
> Diffs
> -
> 
>   pom.xml a4f2bcc14855a2e5f712f9c56fbdb1e137edfa93 
>   sentry-binding/sentry-binding-solr/pom.xml 
> 8b94c87118841c2ce775c3e79ff6bb43ba5185fd 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/hadoop/hdfs/server/namenode/AuthorizationProvider.java
>  383d64d88c74c93cd79ccb189301b9c1a6d919e4 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationConstants.java
>  883680182fd01f4b0b1a7fb5d8eb4d7f23f5851e 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
>  f639f5fbacc85b2adb757087c2eba66b092743a2 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/MockSentryAuthorizationProvider.java
>  2085b525db0b7a9fda77a4be62026da3bf74b7ac 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
>  5da0dc2fb4a47098bd5f9b36dbacb0296162f95b 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  60085b20774fb28c978f60b97bbef0ecbadb8d8f 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  25d13d62a1a16beba16ae46fbd28f2490fca10bc 
> 
> Diff: https://reviews.apache.org/r/50474/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Re: Review Request 50611: Sentry HA Test: programmatic failover, validate privileges before and afterwards.

[Anne Yu]

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

(Updated Aug. 16, 2016, 6:17 p.m.)


Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Changes
---

Addressed previous comments;
Also updated a couple of files to test sentry ha and pool based clients.


Bugs: SENTRY-1423
https://issues.apache.org/jira/browse/SENTRY-1423


Repository: sentry


Description
---

Sentry HA Test: test programmatic failover, validate privileges before and 
afterwards. validate expected exception is thrown.


Diffs (updated)
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
 56d774b883914db438101f47fb94518c94d5 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestProgrammaticFailover.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 7dc3d0f30583b9edd16015218b199e298d192e8c 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImportExport.java
 3f57a003903961a6aea98bd583a14b65bd2b98a2 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
 054b1935975368db058791102f3892f027e31636 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
 dac11517f36a743dff8730465fbf2397ebf16c88 

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


Testing
---


Thanks,

Anne Yu

Re: Review Request 50611: Sentry HA Test: programmatic failover, validate privileges before and afterwards.

[Anne Yu]

> On Aug. 17, 2016, 2:48 a.m., Dapeng Sun wrote:
> > Thank Anne for your patch, I left some minor comments.

Thanks Dapeng.


> On Aug. 17, 2016, 2:48 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java,
> >  line 114
> > <https://reviews.apache.org/r/50611/diff/3/?file=1475619#file1475619line114>
> >
> > Nit: seem it is isActive's opposite, could we remove it to keep the 
> > code clean?

Yeah, maybe just wrap around isActive.


> On Aug. 17, 2016, 2:48 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java,
> >  line 200
> > <https://reviews.apache.org/r/50611/diff/3/?file=1475619#file1475619line200>
> >
> > Nit: Just curious about why externalSentryService is related to  
> > Kerberos keytab file

If the cluster is managed by an external software such as CDH, the keytab file 
is pre-installed, not managed by test. So from outside, we will need to pass 
keytab file location. One qq here, in mini-cluster testing environment 
(internal server), does it also use keytab and kerberos file?


> On Aug. 17, 2016, 2:48 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java,
> >  line 254
> > <https://reviews.apache.org/r/50611/diff/3/?file=1475619#file1475619line254>
> >
> > Nit: Better to add try catch block here.

Will do so.


- Anne


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


On Aug. 16, 2016, 6:17 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50611/
> ---
> 
> (Updated Aug. 16, 2016, 6:17 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1423
> https://issues.apache.org/jira/browse/SENTRY-1423
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Sentry HA Test: test programmatic failover, validate privileges before and 
> afterwards. validate expected exception is thrown.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
>  56d774b883914db438101f47fb94518c94d5 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestProgrammaticFailover.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  7dc3d0f30583b9edd16015218b199e298d192e8c 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImportExport.java
>  3f57a003903961a6aea98bd583a14b65bd2b98a2 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
>  054b1935975368db058791102f3892f027e31636 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
>  dac11517f36a743dff8730465fbf2397ebf16c88 
> 
> Diff: https://reviews.apache.org/r/50611/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 50611: Sentry HA Test: programmatic failover, validate privileges before and afterwards.

[Anne Yu]

> On Aug. 18, 2016, 2:29 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java,
> >  line 200
> > <https://reviews.apache.org/r/50611/diff/3/?file=1475619#file1475619line200>
> >
> > >>One qq here, in mini-cluster testing environment (internal server), 
> > does it also use keytab and kerberos file?
> > 
> > In mini-cluster, I think there is no kerberos at e2e. In some UT, we 
> > use keytab to login and get the credential.
> > 
> > Do you mean the exception is used for avoiding the method is called at 
> > external SentryService case?

I meant to ask to test sentry client in UT, need to consider kerberos 
authentication or not. Maybe not necessary since I don't see it gets configured 
in AbstractTestWithStaticConfiguration.


- Anne


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


On Aug. 16, 2016, 6:17 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50611/
> ---
> 
> (Updated Aug. 16, 2016, 6:17 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1423
> https://issues.apache.org/jira/browse/SENTRY-1423
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Sentry HA Test: test programmatic failover, validate privileges before and 
> afterwards. validate expected exception is thrown.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
>  56d774b883914db438101f47fb94518c94d5 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestProgrammaticFailover.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestSentryHABase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  7dc3d0f30583b9edd16015218b199e298d192e8c 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImportExport.java
>  3f57a003903961a6aea98bd583a14b65bd2b98a2 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
>  054b1935975368db058791102f3892f027e31636 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
>  dac11517f36a743dff8730465fbf2397ebf16c88 
> 
> Diff: https://reviews.apache.org/r/50611/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Review Request 51353: SENTRY-1447: fix a couple of e2e test failures on real clusters.

[Anne Yu]

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

Review request for sentry and Hao Hao.


Bugs: SENTRY-1447
https://issues.apache.org/jira/browse/SENTRY-1447


Repository: sentry


Description
---

We start to see more e2e test failures on s3 clusters. For example,
org.apache.sentry.tests.e2e.hdfs.TestDbHdfsExtMaxGroups need to be ignored for 
Sentry on S3;
If run 
org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs.TestCreateTable, 
have to correctly configure "-Dsentry.e2etest.storage.uri=s3a://sentry-s3";
Add exec(statement, "set hive.exim.uri.scheme.whitelist=hdfs,pfile,s3a;") for 
import/export tests.
Also fixed on long test class causes time out failure.


Diffs
-

  sentry-tests/sentry-tests-hive/pom.xml 
2979b45810b04665c2c66275a0b8544b5f2065d8 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java
 8c152576d452b3069258e7f95c93cbf42e34e325 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbCrossOperations.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
 e545c37c182f3d77995323659ae539a8ad1de0cb 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java
 b38eaaf35ec97978fef06fe91e3e5f5e391d4cc3 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java
 7186f1af8159c30064e5ae3b5b1900d4556e1e4a 

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


Testing
---

On a test cluster.


Thanks,

Anne Yu

Re: Review Request 51353: SENTRY-1447: fix a couple of e2e test failures on real clusters.

[Anne Yu]

> On Aug. 24, 2016, 1:54 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java,
> >  line 134
> > <https://reviews.apache.org/r/51353/diff/1/?file=1482631#file1482631line134>
> >
> > Could you please comment on why we need to set this property?

Import/Export works with s3 storage system only when this is turned on. 
CDH-42473. I will also comment in the code.


- Anne


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


On Aug. 23, 2016, 11:42 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51353/
> ---
> 
> (Updated Aug. 23, 2016, 11:42 p.m.)
> 
> 
> Review request for sentry and Hao Hao.
> 
> 
> Bugs: SENTRY-1447
> https://issues.apache.org/jira/browse/SENTRY-1447
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We start to see more e2e test failures on s3 clusters. For example,
> org.apache.sentry.tests.e2e.hdfs.TestDbHdfsExtMaxGroups need to be ignored 
> for Sentry on S3;
> If run 
> org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs.TestCreateTable, 
> have to correctly configure "-Dsentry.e2etest.storage.uri=s3a://sentry-s3";
> Add exec(statement, "set hive.exim.uri.scheme.whitelist=hdfs,pfile,s3a;") for 
> import/export tests.
> Also fixed on long test class causes time out failure.
> 
> 
> Diffs
> -
> 
>   sentry-tests/sentry-tests-hive/pom.xml 
> 2979b45810b04665c2c66275a0b8544b5f2065d8 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java
>  8c152576d452b3069258e7f95c93cbf42e34e325 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbCrossOperations.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
>  e545c37c182f3d77995323659ae539a8ad1de0cb 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java
>  b38eaaf35ec97978fef06fe91e3e5f5e391d4cc3 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java
>  7186f1af8159c30064e5ae3b5b1900d4556e1e4a 
> 
> Diff: https://reviews.apache.org/r/51353/diff/
> 
> 
> Testing
> ---
> 
> On a test cluster.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51353: SENTRY-1447: fix a couple of e2e test failures on real clusters.

[Anne Yu]

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

(Updated Aug. 24, 2016, 5:07 p.m.)


Review request for sentry and Hao Hao.


Changes
---

Added some comments to address previous review.


Bugs: SENTRY-1447
https://issues.apache.org/jira/browse/SENTRY-1447


Repository: sentry


Description
---

We start to see more e2e test failures on s3 clusters. For example,
org.apache.sentry.tests.e2e.hdfs.TestDbHdfsExtMaxGroups need to be ignored for 
Sentry on S3;
If run 
org.apache.sentry.tests.e2e.hive.fs.TestHiveWarehouseOnExtFs.TestCreateTable, 
have to correctly configure "-Dsentry.e2etest.storage.uri=s3a://sentry-s3";
Add exec(statement, "set hive.exim.uri.scheme.whitelist=hdfs,pfile,s3a;") for 
import/export tests.
Also fixed on long test class causes time out failure.


Diffs (updated)
-

  sentry-tests/sentry-tests-hive/pom.xml 
2979b45810b04665c2c66275a0b8544b5f2065d8 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java
 8c152576d452b3069258e7f95c93cbf42e34e325 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbCrossOperations.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java
 e545c37c182f3d77995323659ae539a8ad1de0cb 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java
 b38eaaf35ec97978fef06fe91e3e5f5e391d4cc3 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java
 7186f1af8159c30064e5ae3b5b1900d4556e1e4a 

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


Testing
---

On a test cluster.


Thanks,

Anne Yu

Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

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

Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Bugs: SENTRY-1429 and SENTRY-1455
https://issues.apache.org/jira/browse/SENTRY-1429
https://issues.apache.org/jira/browse/SENTRY-1455


Repository: sentry


Description
---

SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
test class, improve its cleanUp and setUp for temp HDFS dirs; divide test class 
into smoke test class and p2 test class; in post commit run, we will only run 
smoke tests.


Diffs
-

  pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
 be6d0822ea56509ae9171d2b79026b1c952c2ec9 

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


Testing
---

Local.


Thanks,

Anne Yu

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

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

(Updated Aug. 27, 2016, 12:13 a.m.)


Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Changes
---

Add solr tests back into post commit run profile.


Bugs: SENTRY-1429 and SENTRY-1455
https://issues.apache.org/jira/browse/SENTRY-1429
https://issues.apache.org/jira/browse/SENTRY-1455


Repository: sentry


Description
---

SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
test class, improve its cleanUp and setUp for temp HDFS dirs; divide test class 
into smoke test class and p2 test class; in post commit run, we will only run 
smoke tests.


Diffs (updated)
-

  pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
 be6d0822ea56509ae9171d2b79026b1c952c2ec9 

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


Testing
---

Local.


Thanks,

Anne Yu

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

> On Aug. 27, 2016, 12:24 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  lines 437-453
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line437>
> >
> > So this is the new improvement right?

Thanks for reviewing. Correct, to ensure for each test external dirs can be 
re-created without correct properties.


- Anne


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


On Aug. 27, 2016, 12:13 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 27, 2016, 12:13 a.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1455
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1455
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

> On Aug. 27, 2016, 12:24 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  lines 437-453
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line437>
> >
> > So this is the new improvement right?
> 
> Anne Yu wrote:
> Thanks for reviewing. Correct, to ensure for each test external dirs can 
> be re-created without correct properties.

Typo, "ensure for each test external dirs can be re-created with correct 
properties."


- Anne


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


On Aug. 27, 2016, 12:13 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 27, 2016, 12:13 a.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1455
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1455
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

> On Aug. 27, 2016, 12:30 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 445
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line445>
> >
> > Why are we assuming true here? If the mkdir failed then the setup 
> > essentially fails, failing the test as well isnt it?
> 
> Sravya Tirukkovalur wrote:
> My bad, just checked the semantics of assumeTrue, so we are basically 
> skipping if mkdir fails. That seems fair enough.

I will put assert instead of assume here.


> On Aug. 27, 2016, 12:30 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 447
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line447>
> >
> > Why is this 777? Seems too open for a security test.

Not sure why originally most tests configured it as 777. Will change it to 
drwxrwx---.


> On Aug. 27, 2016, 12:30 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 452
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line452>
> >
> > Same here. Why assume.true?

Will change it back assert.


- Anne


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


On Aug. 27, 2016, 12:13 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 27, 2016, 12:13 a.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1455
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1455
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

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

(Updated Aug. 27, 2016, 12:42 a.m.)


Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Changes
---

Addressed previous code review comments.


Bugs: SENTRY-1429 and SENTRY-1455
https://issues.apache.org/jira/browse/SENTRY-1429
https://issues.apache.org/jira/browse/SENTRY-1455


Repository: sentry


Description
---

SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
test class, improve its cleanUp and setUp for temp HDFS dirs; divide test class 
into smoke test class and p2 test class; in post commit run, we will only run 
smoke tests.


Diffs (updated)
-

  pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
 be6d0822ea56509ae9171d2b79026b1c952c2ec9 

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


Testing
---

Local.


Thanks,

Anne Yu

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

> On Aug. 27, 2016, 12:39 a.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 440
> > <https://reviews.apache.org/r/51471/diff/2/?file=1487247#file1487247line440>
> >
> > Do we really want to create a partition location in setup function? Not 
> > all tests need this right?

But half of them need this partition path. It will be duplicate to create 
partition dir in each of test, for new tests, we might forget to create it. 

BTW, it's time-consuming to create a patch then submit to sentry jira to pass 
hadoop-qa. So will leave this review for a while until get all the reviewers's 
comments. Thanks.


- Anne


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


On Aug. 27, 2016, 12:42 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 27, 2016, 12:42 a.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1455
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1455
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

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

(Updated Aug. 29, 2016, 6:41 p.m.)


Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Changes
---

Addressed Haohao's comments.


Bugs: SENTRY-1429 and SENTRY-1454
https://issues.apache.org/jira/browse/SENTRY-1429
https://issues.apache.org/jira/browse/SENTRY-1454


Repository: sentry


Description
---

SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
test class, improve its cleanUp and setUp for temp HDFS dirs; divide test class 
into smoke test class and p2 test class; in post commit run, we will only run 
smoke tests.


Diffs (updated)
-

  pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 a2aead0c55cbee8aca9608539ad2681891acf57d 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
 b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
 be6d0822ea56509ae9171d2b79026b1c952c2ec9 

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


Testing
---

Local.


Thanks,

Anne Yu

Re: Review Request 51471: SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration and unblock post-commit job.

[Anne Yu]

> On Aug. 29, 2016, 6:22 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java,
> >  line 447
> > <https://reviews.apache.org/r/51471/diff/2-3/?file=1487247#file1487247line447>
> >
> > User needs to have execute bit set to get to the children. So we will 
> > need 771.

Sure will change it. Thanks.


- Anne


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


On Aug. 29, 2016, 6:41 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51471/
> ---
> 
> (Updated Aug. 29, 2016, 6:41 p.m.)
> 
> 
> Review request for sentry, Hao Hao and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-1429 and SENTRY-1454
> https://issues.apache.org/jira/browse/SENTRY-1429
> https://issues.apache.org/jira/browse/SENTRY-1454
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1429, SENTRY-1455: fix flaky TestHDFSIntegration, refactor this huge 
> test class, improve its cleanUp and setUp for temp HDFS dirs; divide test 
> class into smoke test class and p2 test class; in post commit run, we will 
> only run smoke tests.
> 
> 
> Diffs
> -
> 
>   pom.xml b53e7766749c75f8e36097f624fd7a812c6d1761 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  a2aead0c55cbee8aca9608539ad2681891acf57d 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java
>  b0a3b6edc6201a9f902df1786f591b6bf3ac2250 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
>  be6d0822ea56509ae9171d2b79026b1c952c2ec9 
> 
> Diff: https://reviews.apache.org/r/51471/diff/
> 
> 
> Testing
> ---
> 
> Local.
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Review Request 51773: Simple fix for org.apache.sentry.tests.e2e.dbprovider.TestDbUriPermissions.testAlterPartitionLocationPrivileges

[Anne Yu]

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

Review request for sentry and Hao Hao.


Bugs: SENTRY-1464
https://issues.apache.org/jira/browse/SENTRY-1464


Repository: sentry


Description
---

Fixed UnmanagedHiveServer2 parse property API so it doesn't always add prefix 
"hdfs" in Hivewarehouse Uri; 
Print out queries so that can debug straightforward what's going wrong with 
most recent test failures on s3 cluster.


Diffs
-

  
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java
 524fa1c9db4c466e7aa2a8b16b426a61abad31bf 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbCrossOperations.java
 62c0f58dc093df09404c9e12a4203120d420fc6c 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 ba29ca39a4e457f628a12e7f49b8d8dbe04341fc 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
 a92d8c9c75110c54963e0b2ffca3b7e44548e9e4 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java
 a02a156d6fbfc49df7d248bc4258510c48395ee2 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperationsPart2.java
 e11a817f2a41ceb67e8b3d8fe7ea231dbba7f968 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java
 202173c14366e57c7f9109b9e0c75a2213cfbc63 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java
 a2850b987d8734a69291a7fb90b0e95ce7143618 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/TestHiveWarehouseOnExtFs.java
 ed965c1ec4639ba42175a61cd1087b928d7bb922 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
 d5c5db40a9d42140c1f55d3665fd152151d7285e 

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


Testing
---

On a real cluster.


Thanks,

Anne Yu

Re: Review Request 52150: SENTRY-1478: Disable fencing in Sentry store for Active/Active

[Anne Yu]

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



Not mean to object check-in this patch. Just a question, after this patch, 
server.isActive() will also be disabled? In that case, how can one 
programmatically disabe any active server(s) and get the status of any 
server(s)? It seems a userful tool for future and internal testing/debugging. 
Thanks.

- Anne Yu


On Sept. 22, 2016, 5:27 a.m., Li Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52150/
> ---
> 
> (Updated Sept. 22, 2016, 5:27 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Anne Yu, Hao Hao, and Sravya 
> Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Keep the Fencer and Activator classes and disable fencing that brought by 
> SENTRY-1317 and SENTRY-1399, including some tests cases (e.g. TestActivator).
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  6f8239dcf2ae23962efe6bb0edf12d3c14e1a038 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>  39e55c3d9cb17ed498aaad98f1c490401c12030b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e2b92f919c8d4f2bb347ee70c5c623559d57002a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  894fcc966b511ccf309599fd10960f9a11ae8e96 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java
>  353d461d84eb8438506e4bb209828feb98183fa7 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  74977193760dd1958bdafaa3d6dac1ba27d81f32 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java
>  c52197f1ca514604a01ce4f4d9d7d35de30994a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
>  fd10a7a23bcc21786323a27f36aba918f77e819c 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java
>  42b67ba4078d998a069aa6600b1ab24011b7dd26 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  06342063965a0e6b295fbdbaf2dfc517a2045d60 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java
>  98f18314d387a831707ba5f06836ae41926878db 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java
>  e401859ab492c1d2a7634361fa301b3350661929 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  60cfc735832433fb4dbeae1c2d617dd713fc3f3e 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  57d579cc6179319e3525424d652d224323e21457 
> 
> Diff: https://reviews.apache.org/r/52150/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Li Li
> 
>

Re: Review Request 52150: SENTRY-1478: Disable fencing in Sentry store for Active/Active

[Anne Yu]

> On Sept. 22, 2016, 7:56 p.m., Hao Hao wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java,
> >  line 97
> > <https://reviews.apache.org/r/52150/diff/1/?file=1507892#file1507892line97>
> >
> > I think we may still want to keep the activator code, and just disable 
> > fencing in sentry store. So we can have a backup as active/passive model.
> 
> Li Li wrote:
> you are right. Even for active / active mode, we still have only 1 deamon 
> that fetch the snapshot from hms and write to backend db, so we'd Besides, we 
> can keep the activator code. Besides, we can use it for internal testing / 
> debugging that resolve Anne the concern.

Thanks! Yes.


- Anne


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


On Sept. 22, 2016, 5:27 a.m., Li Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52150/
> ---
> 
> (Updated Sept. 22, 2016, 5:27 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Anne Yu, Hao Hao, and Sravya 
> Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Keep the Fencer and Activator classes and disable fencing that brought by 
> SENTRY-1317 and SENTRY-1399, including some tests cases (e.g. TestActivator).
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  6f8239dcf2ae23962efe6bb0edf12d3c14e1a038 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>  39e55c3d9cb17ed498aaad98f1c490401c12030b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e2b92f919c8d4f2bb347ee70c5c623559d57002a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  894fcc966b511ccf309599fd10960f9a11ae8e96 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java
>  353d461d84eb8438506e4bb209828feb98183fa7 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  74977193760dd1958bdafaa3d6dac1ba27d81f32 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java
>  c52197f1ca514604a01ce4f4d9d7d35de30994a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
>  fd10a7a23bcc21786323a27f36aba918f77e819c 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java
>  42b67ba4078d998a069aa6600b1ab24011b7dd26 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  06342063965a0e6b295fbdbaf2dfc517a2045d60 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java
>  98f18314d387a831707ba5f06836ae41926878db 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java
>  e401859ab492c1d2a7634361fa301b3350661929 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  60cfc735832433fb4dbeae1c2d617dd713fc3f3e 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  57d579cc6179319e3525424d652d224323e21457 
> 
> Diff: https://reviews.apache.org/r/52150/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Li Li
> 
>

Review Request 52582: SENTRY-1489: Categorize e2e tests into slow and regular tests, so that can adapt the timeout and etc.

[Anne Yu]

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

Review request for sentry, Hao Hao, Li Li, and Sravya Tirukkovalur.


Bugs: SENTRY-1489
https://issues.apache.org/jira/browse/SENTRY-1489


Repository: sentry


Description
---

org.apache.sentry.tests.e2e.dbprovider.TestDbCrossOperations and some other 
tests occasionally fail with time out exception. Should make a more flexible 
rule for different type of tests. By nature, some tests just take longer time 
to finish.


Diffs
-

  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java
 e7cccbfb7955a497f485e3b69e9dc4d9db191473 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbCrossOperations.java
 60812cfa82d318ddfad32e439d3895f49ed21180 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java
 a314c0dde9c60e7dbb51156cea13980bf8600bf1 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 62da0256e8546a25f0ed0a97b8f3c21eec4fecaa 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/RulesForE2ETest.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/SlowE2ETest.java
 PRE-CREATION 

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


Testing
---


Thanks,

Anne Yu

Re: Review Request 45859: SENTRY-1120: Show role / privileges info in Sentry Service Webpage

[Anne Yu]

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


Fix it, then Ship it!





sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAdminServlet.java
 (line 18)
<https://reviews.apache.org/r/45859/#comment220366>

Just a minor issue. I've seen across codes, license is on usually the top 
then package definition and imports followed afterwards. 


https://github.com/apache/hive/blob/master/common/src/java/org/apache/hadoop/hive/common/FileUtils.java


- Anne Yu


On Oct. 7, 2016, 12:38 a.m., Li Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45859/
> ---
> 
> (Updated Oct. 7, 2016, 12:38 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Anne Yu, Hao Hao, Lenni Kuff, 
> and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Show role / privileges info in Sentry Service Webpage. Since it is only used 
> for debug / test currently, this webpage can be seen only when 
> SENTRY_WEB_ADMIN_SERVLET_ENABLED is true.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  5ec364c460e74d0a9dae8a28c20042360157b8a0 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAdminServlet.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java
>  a42f395270996da345ce49edca909e0438383759 
> 
> Diff: https://reviews.apache.org/r/45859/diff/
> 
> 
> Testing
> ---
> 
> Already tested in kerberos cluster. When 
> sentry.service.web.authentication.type is set to KERBEROS, only the 
> SENTRY_WEB_SECURITY_ALLOW_CONNECT_USERS can see this page. Also this webpage 
> can be seen only when SENTRY_WEB_ADMIN_SERVLET_ENABLED is true.
> 
> 
> Thanks,
> 
> Li Li
> 
>

Re: Review Request 45859: SENTRY-1120: Show role / privileges info in Sentry Service Webpage

[Anne Yu]

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


Ship it!




Thanks Lili for your patch.

- Anne Yu


On Oct. 7, 2016, 5:36 p.m., Li Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45859/
> ---
> 
> (Updated Oct. 7, 2016, 5:36 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Anne Yu, Hao Hao, Lenni Kuff, 
> and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Show role / privileges info in Sentry Service Webpage. Since it is only used 
> for debug / test currently, this webpage can be seen only when 
> SENTRY_WEB_ADMIN_SERVLET_ENABLED is true.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  5ec364c460e74d0a9dae8a28c20042360157b8a0 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAdminServlet.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java
>  a42f395270996da345ce49edca909e0438383759 
> 
> Diff: https://reviews.apache.org/r/45859/diff/
> 
> 
> Testing
> ---
> 
> Already tested in kerberos cluster. When 
> sentry.service.web.authentication.type is set to KERBEROS, only the 
> SENTRY_WEB_SECURITY_ALLOW_CONNECT_USERS can see this page. Also this webpage 
> can be seen only when SENTRY_WEB_ADMIN_SERVLET_ENABLED is true.
> 
> 
> Thanks,
> 
> Li Li
> 
>

Re: Review Request 52582: SENTRY-1489: Categorize e2e tests into slow and regular tests, so that can adapt the timeout and etc.

[Anne Yu]

> On Oct. 7, 2016, 6:03 p.m., Li Li wrote:
> >

Thank you for the review.


> On Oct. 7, 2016, 6:03 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/RulesForE2ETest.java,
> >  line 29
> > <https://reviews.apache.org/r/52582/diff/1/?file=1523454#file1523454line29>
> >
> > It is better to add some some comment for the new class and vars in it.

Yeah, for public class we should do it. Will add.


> On Oct. 7, 2016, 6:03 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/RulesForE2ETest.java,
> >  line 44
> > <https://reviews.apache.org/r/52582/diff/1/?file=1523454#file1523454line44>
> >
> > also 'description != null' ?

Will add.


> On Oct. 7, 2016, 6:03 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/RulesForE2ETest.java,
> >  line 45
> > <https://reviews.apache.org/r/52582/diff/1/?file=1523454#file1523454line45>
> >
> > add debug log for the SlowE2ETest case showing the timeout used?

will add.


> On Oct. 7, 2016, 6:03 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/RulesForE2ETest.java,
> >  line 47
> > <https://reviews.apache.org/r/52582/diff/1/?file=1523454#file1523454line47>
> >
> > use final static var for all the timeout?

will add.


- Anne


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


On Oct. 5, 2016, 11:33 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52582/
> ---
> 
> (Updated Oct. 5, 2016, 11:33 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
> Tirukkovalur.
> 
> 
> Bugs: SENTRY-1489
> https://issues.apache.org/jira/browse/SENTRY-1489
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> org.apache.sentry.tests.e2e.dbprovider.TestDbCrossOperations and some other 
> tests occasionally fail with time out exception. Should make a more flexible 
> rule for different type of tests. By nature, some tests just take longer time 
> to finish.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java
>  e7cccbfb7955a497f485e3b69e9dc4d9db191473 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbCrossOperations.java
>  60812cfa82d318ddfad32e439d3895f49ed21180 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java
>  a314c0dde9c60e7dbb51156cea13980bf8600bf1 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  62da0256e8546a25f0ed0a97b8f3c21eec4fecaa 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/RulesForE2ETest.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/SlowE2ETest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52582/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

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

Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
Tirukkovalur.


Bugs: SENTRY-1497
https://issues.apache.org/jira/browse/SENTRY-1497


Repository: sentry


Description
---

Specify the scale numbers like databases, tables, views, partitions, columns, 
uris, privileges, role, and groups in a config file, the tool can create such 
volume of data in Sentry and HMS databases. To speed up test, it can also do 
the task parallelly.


Diffs
-

  sentry-tests/sentry-tests-hive/pom.xml 
a2512ee3919a3958425f4ab74b178d02e0402315 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
 dcdddeb95a896ca8470d0b994f5460531e34d113 

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


Testing
---


Thanks,

Anne Yu

Re: Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

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

(Updated Oct. 10, 2016, 11:44 p.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
Tirukkovalur.


Changes
---

Added a wrapper class to run all tools from it; will add long haul tests, 
stress tests into the same interface.


Bugs: SENTRY-1497
https://issues.apache.org/jira/browse/SENTRY-1497


Repository: sentry


Description
---

Specify the scale numbers like databases, tables, views, partitions, columns, 
uris, privileges, role, and groups in a config file, the tool can create such 
volume of data in Sentry and HMS databases. To speed up test, it can also do 
the task parallelly.


Diffs (updated)
-

  sentry-tests/sentry-tests-hive/pom.xml 
a2512ee3919a3958425f4ab74b178d02e0402315 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
 90713b1aaa688808859e670c8799f8e5be2d6d26 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
 dcdddeb95a896ca8470d0b994f5460531e34d113 

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


Testing
---


Thanks,

Anne Yu

Re: Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

> On Oct. 10, 2016, 11:33 p.m., Li Li wrote:
> >

Will address these issues. Thanks for reviewing the patch.


- Anne


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


On Oct. 10, 2016, 6:14 a.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52675/
> ---
> 
> (Updated Oct. 10, 2016, 6:14 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
> Tirukkovalur.
> 
> 
> Bugs: SENTRY-1497
> https://issues.apache.org/jira/browse/SENTRY-1497
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Specify the scale numbers like databases, tables, views, partitions, columns, 
> uris, privileges, role, and groups in a config file, the tool can create such 
> volume of data in Sentry and HMS databases. To speed up test, it can also do 
> the task parallelly.
> 
> 
> Diffs
> -
> 
>   sentry-tests/sentry-tests-hive/pom.xml 
> a2512ee3919a3958425f4ab74b178d02e0402315 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
>  dcdddeb95a896ca8470d0b994f5460531e34d113 
> 
> Diff: https://reviews.apache.org/r/52675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

> On Oct. 10, 2016, 11:33 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 102
> > <https://reviews.apache.org/r/52675/diff/1/?file=1529190#file1529190line102>
> >
> > what's the meaning of managed table?

managed table: managed by hive, when drop it's HDFS dirs/files will be dropped 
as well, by default created under /user/hive/warehouse, user can also specify 
location;
external table: managed by user, when drop it's HDFS dirs/files will be kept, 
user specify location to let the table store data in that dir.
http://stackoverflow.com/questions/17038414/difference-between-hive-internal-tables-and-external-tables


> On Oct. 10, 2016, 11:33 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 119
> > <https://reviews.apache.org/r/52675/diff/1/?file=1529190#file1529190line119>
> >
> > since KEYTAB_LOCATION is hdfs keytab, we'd better add hdfs in the 
> > config name. Or we can add another config for the run as user which is hdfs 
> > by default

it's a keytab dir stores all system or user keytab files, can be used by hdfs, 
hive, hbase and others.


> On Oct. 10, 2016, 11:33 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 208
> > <https://reviews.apache.org/r/52675/diff/1/?file=1529190#file1529190line208>
> >
> > what is seq?
> > what if NUM_OF_DATABASES_PER_THREAD - seq - 1 == 0?

seq is index ranging from 0 to a constant number. will add condition to check 
it.


- Anne


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


On Oct. 10, 2016, 11:44 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52675/
> ---
> 
> (Updated Oct. 10, 2016, 11:44 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
> Tirukkovalur.
> 
> 
> Bugs: SENTRY-1497
> https://issues.apache.org/jira/browse/SENTRY-1497
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Specify the scale numbers like databases, tables, views, partitions, columns, 
> uris, privileges, role, and groups in a config file, the tool can create such 
> volume of data in Sentry and HMS databases. To speed up test, it can also do 
> the task parallelly.
> 
> 
> Diffs
> -
> 
>   sentry-tests/sentry-tests-hive/pom.xml 
> a2512ee3919a3958425f4ab74b178d02e0402315 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
>  90713b1aaa688808859e670c8799f8e5be2d6d26 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
>  dcdddeb95a896ca8470d0b994f5460531e34d113 
> 
> Diff: https://reviews.apache.org/r/52675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Re: Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

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

(Updated Oct. 11, 2016, 4:29 a.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
Tirukkovalur.


Changes
---

Addressed Lili's comments; Fixed some threads bugs.


Bugs: SENTRY-1497
https://issues.apache.org/jira/browse/SENTRY-1497


Repository: sentry


Description
---

Specify the scale numbers like databases, tables, views, partitions, columns, 
uris, privileges, role, and groups in a config file, the tool can create such 
volume of data in Sentry and HMS databases. To speed up test, it can also do 
the task parallelly.


Diffs (updated)
-

  sentry-tests/sentry-tests-hive/pom.xml 
a2512ee3919a3958425f4ab74b178d02e0402315 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
 90713b1aaa688808859e670c8799f8e5be2d6d26 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
 dcdddeb95a896ca8470d0b994f5460531e34d113 

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


Testing
---


Thanks,

Anne Yu

Re: [DISCUSS] Add marker for Sentry server log

[Anne Yu]

Hi HaoHao,

Could you elaborate more about what is looking for here? Differentiate
error, exception, failure, warning on server, client side, or system level.
What we want to archive from using marker, the current logger has issues to
indicate problem? Maybe by understanding these requirements can help get
you correct answers.

Thanks for bringing up such discussion.

Cheers,
Anne

On Oct 11, 2016 4:47 PM, "Hao Hao"  wrote:

> Hi all,
>
> Is it a good idea to add different marker for different failures or
> unexpected behaviors for Sentry log? Any recommendation/guideline on what
> kinds of log marker we can have? Thanks a lot!
>
> Best,
> Hao
>

Re: Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

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

(Updated Oct. 18, 2016, 9:53 p.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
Tirukkovalur.


Changes
---

Updated with one more scale test result.


Bugs: SENTRY-1497
https://issues.apache.org/jira/browse/SENTRY-1497


Repository: sentry


Description
---

Specify the scale numbers like databases, tables, views, partitions, columns, 
uris, privileges, role, and groups in a config file, the tool can create such 
volume of data in Sentry and HMS databases. To speed up test, it can also do 
the task parallelly.


Diffs (updated)
-

  sentry-tests/sentry-tests-hive/pom.xml 
a2512ee3919a3958425f4ab74b178d02e0402315 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
 90713b1aaa688808859e670c8799f8e5be2d6d26 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
 dcdddeb95a896ca8470d0b994f5460531e34d113 

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


Testing (updated)
---

Most recent run uses scale configuration similar to capital one pre-prod. 
Totaly running time is 725 secs with 50 threads on a cdh590 cluster. 

Objects status: total databases(300); tables(1224), views(503), 
partitions(5505), columns(1905);
Privileges status: database privileges(299), table privileges(824), view 
privileges(503), partition privileges(204), column privileges(299), uri 
privileges(204); roles(1000), groups(500);
failed threads(1), running time(725 secs)


Thanks,

Anne Yu

Re: Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

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

(Updated Oct. 18, 2016, 10:19 p.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
Tirukkovalur.


Bugs: SENTRY-1497
https://issues.apache.org/jira/browse/SENTRY-1497


Repository: sentry


Description
---

Specify the scale numbers like databases, tables, views, partitions, columns, 
uris, privileges, role, and groups in a config file, the tool can create such 
volume of data in Sentry and HMS databases. To speed up test, it can also do 
the task parallelly.


Diffs
-

  sentry-tests/sentry-tests-hive/pom.xml 
a2512ee3919a3958425f4ab74b178d02e0402315 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
 90713b1aaa688808859e670c8799f8e5be2d6d26 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
 dcdddeb95a896ca8470d0b994f5460531e34d113 

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


Testing (updated)
---

Most recent run uses scale configuration. Totaly running time is 725 secs with 
50 threads on a real cluster. 

Objects status: total databases(300); tables(1224), views(503), 
partitions(5505), columns(1905);
Privileges status: database privileges(299), table privileges(824), view 
privileges(503), partition privileges(204), column privileges(299), uri 
privileges(204); roles(1000), groups(500);
failed threads(1), running time(725 secs)


Thanks,

Anne Yu

Re: Review Request 52675: Create a sentry scale test tool to add various objects and privileges into Sentry and HMS.

[Anne Yu]

> On Oct. 12, 2016, 5:10 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java,
> >  line 33
> > <https://reviews.apache.org/r/52675/diff/3/?file=1530271#file1530271line33>
> >
> > Why make it non final?

Will add it back.


> On Oct. 12, 2016, 5:10 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java,
> >  line 60
> > <https://reviews.apache.org/r/52675/diff/3/?file=1530271#file1530271line60>
> >
> > how about just using: val = System.getProperty(hiveVar, new 
> > Configuration().get(hiveVar)) ?

Yeah, will combine the lines.


> On Oct. 12, 2016, 5:10 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java,
> >  line 69
> > <https://reviews.apache.org/r/52675/diff/3/?file=1530271#file1530271line69>
> >
> > seems it may not be system property, see line 63?

Didn't get it here. val = System.getProperty(...,...), so the comment clarifies 
it as a system property.


> On Oct. 12, 2016, 5:10 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 42
> > <https://reviews.apache.org/r/52675/diff/3/?file=1530272#file1530272line42>
> >
> > run -> running ?

Will change it to Before run.


> On Oct. 12, 2016, 5:10 p.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java,
> >  line 68
> > <https://reviews.apache.org/r/52675/diff/3/?file=1530273#file1530273line68>
> >
> > do we need to add 'else' for all other cases (print help)?

Added in the latest patch.


- Anne


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


On Oct. 18, 2016, 10:19 p.m., Anne Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52675/
> ---
> 
> (Updated Oct. 18, 2016, 10:19 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
> Tirukkovalur.
> 
> 
> Bugs: SENTRY-1497
> https://issues.apache.org/jira/browse/SENTRY-1497
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Specify the scale numbers like databases, tables, views, partitions, columns, 
> uris, privileges, role, and groups in a config file, the tool can create such 
> volume of data in Sentry and HMS databases. To speed up test, it can also do 
> the task parallelly.
> 
> 
> Diffs
> -
> 
>   sentry-tests/sentry-tests-hive/pom.xml 
> a2512ee3919a3958425f4ab74b178d02e0402315 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
>  90713b1aaa688808859e670c8799f8e5be2d6d26 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
>  dcdddeb95a896ca8470d0b994f5460531e34d113 
> 
> Diff: https://reviews.apache.org/r/52675/diff/
> 
> 
> Testing
> ---
> 
> Most recent run uses scale configuration. Totaly running time is 725 secs 
> with 50 threads on a real cluster. 
> 
> Objects status: total databases(300); tables(1224), views(503), 
> partitions(5505), columns(1905);
> Privileges status: database privileges(299), table privileges(824), view 
> privileges(503), partition privileges(204), column privileges(299), uri 
> privileges(204); roles(1000), groups(500);
> failed threads(1), running time(725 secs)
> 
> 
> Thanks,
> 
> Anne Yu
> 
>