Re: HMS catalogs and Sentry

2018-04-25 Thread Alexander Kolbasov
See https://issues.apache.org/jira/browse/HIVE-18755. Basically catalog is
a container for databases - all databases belong to a catalog. This means
that Sentry may need to support catalog-level privileges.

- Alex

On Wed, Apr 25, 2018 at 5:06 PM, Kalyan Kumar Kalvagadda <
kkal...@cloudera.com> wrote:

> Sasha,
>
> Could you share information on what catalog is?
>
> -Kalyan
>
>
> *Thanks,Kalyan Kumar Kalvagadda* | Software Engineer
> t. (469) 279- <00>5732
> cloudera.com 
>
> [image: Cloudera] 
>
> [image: Cloudera on Twitter]  [image:
> Cloudera on Facebook]  [image: Cloudera
> on LinkedIn] 
> --
>
> On Wed, Apr 25, 2018 at 6:58 PM, Alexander Kolbasov 
> wrote:
>
> > Hive 3 introduced the notion of catalog. What do people think about
> > supporting it in Sentry as well? At some point we will move to Hive-3 and
> > then it will become a requirement I guess.
> >
> > - Alex
> >
>


Re: HMS catalogs and Sentry

2018-04-25 Thread Kalyan Kumar Kalvagadda
Sasha,

Could you share information on what catalog is?

-Kalyan


*Thanks,Kalyan Kumar Kalvagadda* | Software Engineer
t. (469) 279- <00>5732
cloudera.com 

[image: Cloudera] 

[image: Cloudera on Twitter]  [image:
Cloudera on Facebook]  [image: Cloudera
on LinkedIn] 
--

On Wed, Apr 25, 2018 at 6:58 PM, Alexander Kolbasov 
wrote:

> Hive 3 introduced the notion of catalog. What do people think about
> supporting it in Sentry as well? At some point we will move to Hive-3 and
> then it will become a requirement I guess.
>
> - Alex
>


HMS catalogs and Sentry

2018-04-25 Thread Alexander Kolbasov
Hive 3 introduced the notion of catalog. What do people think about
supporting it in Sentry as well? At some point we will move to Hive-3 and
then it will become a requirement I guess.

- Alex


Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

2018-04-25 Thread Steve Moist via Review Board


> On April 25, 2018, 7:58 p.m., Sergio Pena wrote:
> > I found out that Hive made a change on the smart-apply-patch.sh to use git 
> > apply instead of patch, and the patch still have some validation to check 
> > if the patch was generated using the --no-prefix and also the levels.
> > See 
> > https://github.com/apache/hive/blob/master/testutils/ptest2/src/main/resources/smart-apply-patch.sh
> > 
> > Should we do something similar? Just replace patch for git apply? or do you 
> > think it is not neccessary?

That looks pretty good, I'll take a look at it and why it's doing that instead 
of a straight git apply.


- Steve


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


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>



Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

2018-04-25 Thread Steve Moist via Review Board


> On April 25, 2018, 4:07 p.m., Anthony Young-Garner wrote:
> > dev-support/smart-apply-patch.sh
> > Line 44 (original)
> > 
> >
> > Do we not still want to detect this situation (only new files) and try 
> > applying the patch at levels 0,1,2? (Lines 44 - 85)
> 
> Steve Moist wrote:
> No, we should just use git apply to be consistant.
> 
> Alexander Kolbasov wrote:
> A lot of people generate patches that are not applyable with git apply.

Do you have an example of it?


- Steve


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


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>



Re: Review Request 66764: SENTRY-2210: AUTHZ_PATH should have index on the foreign key AUTHZ_OBJ_ID

2018-04-25 Thread Na Li via Review Board


> On April 25, 2018, 8:21 p.m., Sergio Pena wrote:
> > Lina, would it be better to create another patch to fix the 2.1.0 versions? 
> > So we can have this patch to add the index only instead of several new 
> > files, what do you think?

Sergio, I don't want to take the trouble to fix the 2.1.0 versions. However, 
2.0 is already released, any schema change on top of that has to mark it as for 
version 2.1.0. Is that correct?


- Na


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


On April 23, 2018, 10:56 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66764/
> ---
> 
> (Updated April 23, 2018, 10:56 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> add index for the foreign key AUTHZ_OBJ_ID on table AUTHZ_PATH
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java
>  f1a5b10 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.derby.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.mysql.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.oracle.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.postgres.sql
>  PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-derby-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-mysql-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-oracle-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-postgres-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.db2 
> 5ae9349 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.derby 
> 770e1b5 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.mysql 
> 770e1b5 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.oracle 
> 770e1b5 
>   
> sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.postgres 
> 770e1b5 
> 
> 
> Diff: https://reviews.apache.org/r/66764/diff/3/
> 
> 
> Testing
> ---
> 
> run mysql script to verify it works on mysql
> SentrySchemaTool upgrade/install tests
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 66764: SENTRY-2210: AUTHZ_PATH should have index on the foreign key AUTHZ_OBJ_ID

2018-04-25 Thread Sergio Pena via Review Board

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



Lina, would it be better to create another patch to fix the 2.1.0 versions? So 
we can have this patch to add the index only instead of several new files, what 
do you think?

- Sergio Pena


On April 23, 2018, 10:56 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66764/
> ---
> 
> (Updated April 23, 2018, 10:56 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> add index for the foreign key AUTHZ_OBJ_ID on table AUTHZ_PATH
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java
>  f1a5b10 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.derby.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.mysql.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.oracle.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/010-SENTRY-2210.postgres.sql
>  PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-2.1.0.sql 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-derby-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-mysql-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-oracle-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-postgres-2.0.0-to-2.1.0.sql
>  PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.db2 
> 5ae9349 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.derby 
> 770e1b5 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.mysql 
> 770e1b5 
>   sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.oracle 
> 770e1b5 
>   
> sentry-provider/sentry-provider-db/src/main/resources/upgrade.order.postgres 
> 770e1b5 
> 
> 
> Diff: https://reviews.apache.org/r/66764/diff/3/
> 
> 
> Testing
> ---
> 
> run mysql script to verify it works on mysql
> SentrySchemaTool upgrade/install tests
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

2018-04-25 Thread Sergio Pena via Review Board

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



I found out that Hive made a change on the smart-apply-patch.sh to use git 
apply instead of patch, and the patch still have some validation to check if 
the patch was generated using the --no-prefix and also the levels.
See 
https://github.com/apache/hive/blob/master/testutils/ptest2/src/main/resources/smart-apply-patch.sh

Should we do something similar? Just replace patch for git apply? or do you 
think it is not neccessary?

- Sergio Pena


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>



Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

2018-04-25 Thread Alexander Kolbasov


> On April 25, 2018, 4:07 p.m., Anthony Young-Garner wrote:
> > dev-support/smart-apply-patch.sh
> > Line 44 (original)
> > 
> >
> > Do we not still want to detect this situation (only new files) and try 
> > applying the patch at levels 0,1,2? (Lines 44 - 85)
> 
> Steve Moist wrote:
> No, we should just use git apply to be consistant.

A lot of people generate patches that are not applyable with git apply.


- Alexander


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


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>



Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

2018-04-25 Thread Steve Moist via Review Board


> On April 25, 2018, 4:51 p.m., Colm O hEigeartaigh wrote:
> > Why is the package name changed for GenericPrivilegeConverter.java + 
> > TSentryPrivilegeConverter.java? They've gone from 
> > "sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools"
> >  to "sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools". 
> > Aren't they only applicable to the generic case and if so wouldn't it make 
> > more sense to leave them in the generic package?

I was moving things around and probably forgot about that one.  they have been 
moved as part of SENTRY-2206 into the api module as they're primarily used 
there.


- Steve


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


On April 24, 2018, 7:29 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66748/
> ---
> 
> (Updated April 24, 2018, 7:29 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Moved the sentry cli to sentry-tools.  Had to change a dependency in 
> sentry-provider-db to re-use the integration base.
> 
> 
> Diffs
> -
> 
>   bin/sentryShell 17b1429f 
>   pom.xml 16a3838a 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
>   sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
>  cf552b16 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
>  edf09346 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>  8de543c4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  e3d81f80 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
>  5735 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  013e824b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
>  a5996a7b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  1a4692e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
>  4487685a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
>  5bbe7727 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
>  a792b5cc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
>  0bfbc442 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
>  cf1c7258 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
>  d75e24bb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
>  c8b2eef3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
>  785e27df 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
>  eeb3a23f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  3f0b5fad 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
>  3abba526 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
>  69c067fe 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
>  4dddf780 
>   
> 

Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

2018-04-25 Thread Steve Moist via Review Board


> On April 25, 2018, 4:07 p.m., Anthony Young-Garner wrote:
> > dev-support/smart-apply-patch.sh
> > Line 33 (original), 32 (patched)
> > 
> >
> > Will we not continue to support stdin patches? I think it can still be 
> > achieved with git.

It can, this is a bit simpler to just read from a file. I can add it back if 
needed.  Does anyone know if it's being used this way?


> On April 25, 2018, 4:07 p.m., Anthony Young-Garner wrote:
> > dev-support/smart-apply-patch.sh
> > Line 44 (original)
> > 
> >
> > Do we not still want to detect this situation (only new files) and try 
> > applying the patch at levels 0,1,2? (Lines 44 - 85)

No, we should just use git apply to be consistant.


- Steve


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


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>



Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

2018-04-25 Thread Colm O hEigeartaigh

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



Why is the package name changed for GenericPrivilegeConverter.java + 
TSentryPrivilegeConverter.java? They've gone from 
"sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools" 
to "sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools". 
Aren't they only applicable to the generic case and if so wouldn't it make more 
sense to leave them in the generic package?

- Colm O hEigeartaigh


On April 24, 2018, 7:29 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66748/
> ---
> 
> (Updated April 24, 2018, 7:29 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Moved the sentry cli to sentry-tools.  Had to change a dependency in 
> sentry-provider-db to re-use the integration base.
> 
> 
> Diffs
> -
> 
>   bin/sentryShell 17b1429f 
>   pom.xml 16a3838a 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
>   sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
>  cf552b16 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
>  edf09346 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>  8de543c4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  e3d81f80 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
>  5735 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  013e824b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
>  a5996a7b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  1a4692e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
>  4487685a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
>  5bbe7727 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
>  a792b5cc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
>  0bfbc442 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
>  cf1c7258 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
>  d75e24bb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
>  c8b2eef3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
>  785e27df 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
>  eeb3a23f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  3f0b5fad 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
>  3abba526 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
>  69c067fe 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
>  4dddf780 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
>  9e6ff421 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellIndexer.java
>  f66eb859 
>   
> 

Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

2018-04-25 Thread Steve Moist via Review Board

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




dev-support/smart-apply-patch.sh
Line 33 (original), 32 (patched)


It can, this is a bit simpler to just read from a file. I can add it back 
if needed.  Does anyone know if it's being used this way?


- Steve Moist


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>



Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

2018-04-25 Thread Anthony Young-Garner via Review Board

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




dev-support/smart-apply-patch.sh
Line 33 (original), 32 (patched)


Will we not continue to support stdin patches? I think it can still be 
achieved with git.



dev-support/smart-apply-patch.sh
Line 44 (original)


Do we not still want to detect this situation (only new files) and try 
applying the patch at levels 0,1,2? (Lines 44 - 85)


- Anthony Young-Garner


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>