You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Brock Noland <br...@cloudera.com> on 2013/12/04 20:05:35 UTC
Review Request 16007: HIVE-5946 - DDL authorization task factory should be
pluggable and tested
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16007/
-----------------------------------------------------------
Review request for hive.
Bugs: HIVE-5946
https://issues.apache.org/jira/browse/HIVE-5946
Repository: hive-git
Description
-------
Moves the task creation logic to a factory which is now tested with 20 unit tests.
Diffs
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 36503fa911161f5bca1b50182481a5a6ee38ae42
ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java d0807d23de740ede9abe054aab946f043d6918b0
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java da80d817bb6e97db1d9a12e96a5c736ab2c3222b
ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 7443ea45cb1502090c1f6d03ce0c3b21d0bcaba4
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/AbstractHiveAuthorizationTaskFactory.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/DefaultHiveAuthorizationTaskFactory.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactory.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryFactory.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestDefaultHiveAuthorizationTaskFactory.java PRE-CREATION
Diff: https://reviews.apache.org/r/16007/diff/
Testing
-------
20 additional unit tests which pass
Thanks,
Brock Noland
Re: Review Request 16007: HIVE-5946 - DDL authorization task factory should
be pluggable and tested
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16007/
-----------------------------------------------------------
(Updated Jan. 6, 2014, 4:31 p.m.)
Review request for hive.
Changes
-------
Latest patch.
Bugs: HIVE-5946
https://issues.apache.org/jira/browse/HIVE-5946
Repository: hive-git
Description
-------
Moves the task creation logic to a factory which is now tested with 20 unit tests.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1f242ea
ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java d0807d2
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 448dae2
ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 8bd0927
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactory.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java PRE-CREATION
Diff: https://reviews.apache.org/r/16007/diff/
Testing
-------
20 additional unit tests which pass
Thanks,
Brock Noland
Re: Review Request 16007: HIVE-5946 - DDL authorization task factory should
be pluggable and tested
Posted by Brock Noland <br...@cloudera.com>.
> On Jan. 2, 2014, 9:52 p.m., Thejas Nair wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestDefaultHiveAuthorizationTaskFactory.java, line 430
> > <https://reviews.apache.org/r/16007/diff/2/?file=393936#file393936line430>
> >
> > A nit -
> > I found it a little unintuitive to return the list from assertSize() call. Should we just have return type as void and do the check on another line.
> > I don't have a strong opinion about this, so keeping it this way is also fine.
> >
> >
I created a tiny class to match this fluent without adding a bunch of extra LOC.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16007/#review31067
-----------------------------------------------------------
On Jan. 6, 2014, 4:31 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16007/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2014, 4:31 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-5946
> https://issues.apache.org/jira/browse/HIVE-5946
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Moves the task creation logic to a factory which is now tested with 20 unit tests.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1f242ea
> ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java d0807d2
> ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 448dae2
> ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 8bd0927
> ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactory.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16007/diff/
>
>
> Testing
> -------
>
> 20 additional unit tests which pass
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 16007: HIVE-5946 - DDL authorization task factory should
be pluggable and tested
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16007/#review31067
-----------------------------------------------------------
ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestDefaultHiveAuthorizationTaskFactory.java
<https://reviews.apache.org/r/16007/#comment59407>
A nit -
I found it a little unintuitive to return the list from assertSize() call. Should we just have return type as void and do the check on another line.
I don't have a strong opinion about this, so keeping it this way is also fine.
- Thejas Nair
On Dec. 4, 2013, 7:05 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16007/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2013, 7:05 p.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-5946
> https://issues.apache.org/jira/browse/HIVE-5946
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Moves the task creation logic to a factory which is now tested with 20 unit tests.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 36503fa911161f5bca1b50182481a5a6ee38ae42
> ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java d0807d23de740ede9abe054aab946f043d6918b0
> ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java da80d817bb6e97db1d9a12e96a5c736ab2c3222b
> ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 7443ea45cb1502090c1f6d03ce0c3b21d0bcaba4
> ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/AbstractHiveAuthorizationTaskFactory.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/DefaultHiveAuthorizationTaskFactory.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactory.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryFactory.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestDefaultHiveAuthorizationTaskFactory.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16007/diff/
>
>
> Testing
> -------
>
> 20 additional unit tests which pass
>
>
> Thanks,
>
> Brock Noland
>
>