You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Reuben Kuhnert <si...@gmail.com> on 2015/04/29 18:09:29 UTC

Review Request 33680: Create new hive-site property for supporting port configuration

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

Review request for hive and Sergio Pena.


Bugs: HIVE-9365
    https://issues.apache.org/jira/browse/HIVE-9365


Repository: hive-git


Description
-------

HIVE-9365: The Metastore should take port configuration from hive-site.xml


Diffs
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c9ee423cf73706feb2774dacca9fc7b94fe80617 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java PRE-CREATION 

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


Testing
-------

Unit tests attached. Also tested locally.


Thanks,

Reuben Kuhnert


Re: Review Request 33680: Create new hive-site property for supporting port configuration

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33680/#review81986
-----------------------------------------------------------



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/33680/#comment132550>

    I think we should leave this legacy code. Other users might be using the METASTORE_PORT environment variable.



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/33680/#comment132551>

    Should we follow the generic java convention name for a getter method?
    
    It states that 'getPort' should be the name. It makes the code readable.



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java
<https://reviews.apache.org/r/33680/#comment132554>

    Please add the APACHE license at the head of the file.



metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java
<https://reviews.apache.org/r/33680/#comment132556>

    Explain what this class does instead of adding who created it. 
    
    I haven't seen hive code that mentions the author. I don't know if it is a hive rule to not write the author name. Let's just remove to follow its rules.


- Sergio Pena


On April 29, 2015, 4:09 p.m., Reuben Kuhnert wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33680/
> -----------------------------------------------------------
> 
> (Updated April 29, 2015, 4:09 p.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Bugs: HIVE-9365
>     https://issues.apache.org/jira/browse/HIVE-9365
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9365: The Metastore should take port configuration from hive-site.xml
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c9ee423cf73706feb2774dacca9fc7b94fe80617 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33680/diff/
> 
> 
> Testing
> -------
> 
> Unit tests attached. Also tested locally.
> 
> 
> Thanks,
> 
> Reuben Kuhnert
> 
>


Re: Review Request 33680: Create new hive-site property for supporting port configuration

Posted by Lefty Leverenz <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33680/#review82091
-----------------------------------------------------------



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/33680/#comment132758>

    tiny nit:  you don't need to capitalize metastore (but this isn't worth a separate patch, "Metastore" is okay too, so only change it if you're making other changes)


- Lefty Leverenz


On April 29, 2015, 11:35 a.m., Reuben Kuhnert wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33680/
> -----------------------------------------------------------
> 
> (Updated April 29, 2015, 11:35 a.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Bugs: HIVE-9365
>     https://issues.apache.org/jira/browse/HIVE-9365
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9365: The Metastore should take port configuration from hive-site.xml
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c9ee423cf73706feb2774dacca9fc7b94fe80617 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33680/diff/
> 
> 
> Testing
> -------
> 
> Unit tests attached. Also tested locally.
> 
> 
> Thanks,
> 
> Reuben Kuhnert
> 
>


Re: Review Request 33680: Create new hive-site property for supporting port configuration

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33680/#review82381
-----------------------------------------------------------

Ship it!


Ship It!

- Sergio Pena


On May 4, 2015, 12:30 p.m., Reuben Kuhnert wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33680/
> -----------------------------------------------------------
> 
> (Updated May 4, 2015, 12:30 p.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Bugs: HIVE-9365
>     https://issues.apache.org/jira/browse/HIVE-9365
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9365: The Metastore should take port configuration from hive-site.xml
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c9ee423cf73706feb2774dacca9fc7b94fe80617 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33680/diff/
> 
> 
> Testing
> -------
> 
> Unit tests attached. Also tested locally.
> 
> 
> Thanks,
> 
> Reuben Kuhnert
> 
>


Re: Review Request 33680: Create new hive-site property for supporting port configuration

Posted by Lefty Leverenz <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33680/#review82450
-----------------------------------------------------------

Ship it!


Ship It!

- Lefty Leverenz


On May 4, 2015, 5:30 a.m., Reuben Kuhnert wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33680/
> -----------------------------------------------------------
> 
> (Updated May 4, 2015, 5:30 a.m.)
> 
> 
> Review request for hive and Sergio Pena.
> 
> 
> Bugs: HIVE-9365
>     https://issues.apache.org/jira/browse/HIVE-9365
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9365: The Metastore should take port configuration from hive-site.xml
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c9ee423cf73706feb2774dacca9fc7b94fe80617 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33680/diff/
> 
> 
> Testing
> -------
> 
> Unit tests attached. Also tested locally.
> 
> 
> Thanks,
> 
> Reuben Kuhnert
> 
>


Re: Review Request 33680: Create new hive-site property for supporting port configuration

Posted by Reuben Kuhnert <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33680/
-----------------------------------------------------------

(Updated 5 4, 2015, 12:30 오후)


Review request for hive and Sergio Pena.


Changes
-------

Updated with Lefty's suggestions


Bugs: HIVE-9365
    https://issues.apache.org/jira/browse/HIVE-9365


Repository: hive-git


Description
-------

HIVE-9365: The Metastore should take port configuration from hive-site.xml


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c9ee423cf73706feb2774dacca9fc7b94fe80617 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java PRE-CREATION 

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


Testing
-------

Unit tests attached. Also tested locally.


Thanks,

Reuben Kuhnert


Re: Review Request 33680: Create new hive-site property for supporting port configuration

Posted by Reuben Kuhnert <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33680/
-----------------------------------------------------------

(Updated April 29, 2015, 6:35 p.m.)


Review request for hive and Sergio Pena.


Changes
-------

Updated with @spena's suggestions


Bugs: HIVE-9365
    https://issues.apache.org/jira/browse/HIVE-9365


Repository: hive-git


Description
-------

HIVE-9365: The Metastore should take port configuration from hive-site.xml


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c9ee423cf73706feb2774dacca9fc7b94fe80617 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 3f267ff0eb20560c36a19b74353f9d6749c8b333 
  metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetastoreCli.java PRE-CREATION 

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


Testing
-------

Unit tests attached. Also tested locally.


Thanks,

Reuben Kuhnert