You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Anant Nag <nn...@gmail.com> on 2015/04/08 09:13:14 UTC
Review Request 32966: HIVE-10251: HIVE-9664 makes hive depend on
ivysettings.xml
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32966/
-----------------------------------------------------------
Review request for hive.
Bugs: HIVE-10251
https://issues.apache.org/jira/browse/HIVE-10251
Repository: hive-git
Description
-------
* DependencyResolver is now not initialized in constructor.
* Changed all methods of DependencyResolver to static and added static initialization block
* Added additional checks for ivysettings.xml file. If it is not found in classpath then defaultGrapeConfig.xml is used instead.
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java 27bf3e46291e02bd689d7f3ac2cb5e71a18c8b2b
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 66cff87d118ee22f2e8308b0b5c70f87d99d5625
Diff: https://reviews.apache.org/r/32966/diff/
Testing
-------
Thanks,
Anant Nag
Re: Review Request 32966: HIVE-10251: HIVE-9664 makes hive depend on
ivysettings.xml
Posted by Swarnim Kulkarni <ku...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32966/#review79344
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java
<https://reviews.apache.org/r/32966/#comment128610>
You would reach here only if both ivysettings.xml and defaultGrapeConfig.xml are missing. Should the message indicate that as well?
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java
<https://reviews.apache.org/r/32966/#comment128608>
Seems like this should be something more severe than just an INFO. A WARN maybe?
- Swarnim Kulkarni
On April 8, 2015, 7:13 a.m., Anant Nag wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32966/
> -----------------------------------------------------------
>
> (Updated April 8, 2015, 7:13 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-10251
> https://issues.apache.org/jira/browse/HIVE-10251
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> * DependencyResolver is now not initialized in constructor.
> * Changed all methods of DependencyResolver to static and added static initialization block
> * Added additional checks for ivysettings.xml file. If it is not found in classpath then defaultGrapeConfig.xml is used instead.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java 27bf3e46291e02bd689d7f3ac2cb5e71a18c8b2b
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 66cff87d118ee22f2e8308b0b5c70f87d99d5625
>
> Diff: https://reviews.apache.org/r/32966/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anant Nag
>
>
Re: Review Request 32966: HIVE-10251: HIVE-9664 makes hive depend on
ivysettings.xml
Posted by Anant Nag <nn...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32966/
-----------------------------------------------------------
(Updated April 10, 2015, 1:34 p.m.)
Review request for hive.
Changes
-------
* Added unit test.
* Moved ivysettings.xml check to a separate function
* Added hive-ivysettings-default.xml file which will be packed in hive-exec jar.
Bugs: HIVE-10251
https://issues.apache.org/jira/browse/HIVE-10251
Repository: hive-git
Description
-------
* DependencyResolver is now not initialized in constructor.
* Changed all methods of DependencyResolver to static and added static initialization block
* Added additional checks for ivysettings.xml file. If it is not found in classpath then defaultGrapeConfig.xml is used instead.
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java b20e975416a8565d5668c509ef62a3ca1c46989a
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 66cff87d118ee22f2e8308b0b5c70f87d99d5625
ql/src/main/resources/org/apache/hadoop/hive/ql/session/hive-ivysettings-default.xml PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/session/TestDependencyResolver.java PRE-CREATION
Diff: https://reviews.apache.org/r/32966/diff/
Testing
-------
Thanks,
Anant Nag
Re: Review Request 32966: HIVE-10251: HIVE-9664 makes hive depend on
ivysettings.xml
Posted by Anant Nag <nn...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32966/
-----------------------------------------------------------
(Updated April 9, 2015, 10:16 a.m.)
Review request for hive.
Changes
-------
* Removed static methods and changed DependencyResolver to singleton class.
* Addressed Swarnim's and Gunther's comments.
Bugs: HIVE-10251
https://issues.apache.org/jira/browse/HIVE-10251
Repository: hive-git
Description
-------
* DependencyResolver is now not initialized in constructor.
* Changed all methods of DependencyResolver to static and added static initialization block
* Added additional checks for ivysettings.xml file. If it is not found in classpath then defaultGrapeConfig.xml is used instead.
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java 27bf3e46291e02bd689d7f3ac2cb5e71a18c8b2b
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 66cff87d118ee22f2e8308b0b5c70f87d99d5625
Diff: https://reviews.apache.org/r/32966/diff/
Testing
-------
Thanks,
Anant Nag
Re: Review Request 32966: HIVE-10251: HIVE-9664 makes hive depend on
ivysettings.xml
Posted by Anant Nag <nn...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32966/#review79475
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java
<https://reviews.apache.org/r/32966/#comment128883>
This class would be used only when someone is trying to add jar from repository. Also other methods of this class depend upon the existence of ivysettings.xml file. So I think we should have this message here if ivysettings is not found.
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java
<https://reviews.apache.org/r/32966/#comment128885>
Sure. I'll include it in the message.
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java
<https://reviews.apache.org/r/32966/#comment128884>
Sure. I'll change it.
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
<https://reviews.apache.org/r/32966/#comment128882>
I changed the methods to static to initialize the class only when it is required by the add jar feature and not initialize every time this feature is used. I think changing it to a singleton class would be a better option than making methods static. I'll do that.
- Anant Nag
On April 8, 2015, 7:13 a.m., Anant Nag wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32966/
> -----------------------------------------------------------
>
> (Updated April 8, 2015, 7:13 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-10251
> https://issues.apache.org/jira/browse/HIVE-10251
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> * DependencyResolver is now not initialized in constructor.
> * Changed all methods of DependencyResolver to static and added static initialization block
> * Added additional checks for ivysettings.xml file. If it is not found in classpath then defaultGrapeConfig.xml is used instead.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java 27bf3e46291e02bd689d7f3ac2cb5e71a18c8b2b
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 66cff87d118ee22f2e8308b0b5c70f87d99d5625
>
> Diff: https://reviews.apache.org/r/32966/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anant Nag
>
>
Re: Review Request 32966: HIVE-10251: HIVE-9664 makes hive depend on
ivysettings.xml
Posted by Gunther Hagleitner <gh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32966/#review79412
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java
<https://reviews.apache.org/r/32966/#comment128742>
is it possible to not print this on the console (logs only)? or at least only print when you find a specific file (the reverse of what you're doing now). there's no reason to alert someone that the file wasn't found if they're not interested in this feature.
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
<https://reviews.apache.org/r/32966/#comment128741>
why are you turning all this into static methods? using actual objects is more testable/maintainable no?
- Gunther Hagleitner
On April 8, 2015, 7:13 a.m., Anant Nag wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32966/
> -----------------------------------------------------------
>
> (Updated April 8, 2015, 7:13 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-10251
> https://issues.apache.org/jira/browse/HIVE-10251
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> * DependencyResolver is now not initialized in constructor.
> * Changed all methods of DependencyResolver to static and added static initialization block
> * Added additional checks for ivysettings.xml file. If it is not found in classpath then defaultGrapeConfig.xml is used instead.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/session/DependencyResolver.java 27bf3e46291e02bd689d7f3ac2cb5e71a18c8b2b
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 66cff87d118ee22f2e8308b0b5c70f87d99d5625
>
> Diff: https://reviews.apache.org/r/32966/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anant Nag
>
>