> On March 29, 2018, 11:32 p.m., Vihang Karajgaonkar wrote: > > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParser.java > > Lines 79 (patched) > > <https://reviews.apache.org/r/66185/diff/2/?file=1989804#file1989804line79> > > > > Most of the code in this file is duplicated from > > UserHS2ConnectionFileParser. This logic would work for what you are trying > > to achieve but I think we are doing this in much more complex way than > > necessary. Here is my suggestion: > > > > Based on my understanding you have following requirements: > > 1. User will provide a named url like eg blue using beeline -c blue > > 2. If the user doesn't provide a named url a default url is used. > > > > You can do this with a small change in the existing code and without > > the need to merge the two connection files. In the example above, if the > > user doesn't specify -c option above but the beeline-hs2-connection.xml > > exists the code is same as what we have currently. (We can obviously > > support beeline-site.xml name as well with a trivial change) > > > > if the user specifies beeline -c blue instead, in > > UserHS2ConnectionFileParser.getConnectionProperties() instead of looking > > for keys starting for beeline.hs2.connection. you can modify the code to > > look for keys starting from beeline.hs2.connection.blue. In that case you > > reuse most of the existing code and don't need the additional merging > > logic. The result is you have only one user connection file and one > > hive-site.xml which can be overriden by user connection file. > > > > The only compromise in the above design is that you cannot provide one > > long url string like you want to do in beeline-site.xml. If this is not a > > hard requirement, I think we will have a much cleaner implementation with > > less confusion between having two connection xml files which behave almost > > the same way. If its a hard requirement, I guess you can still do by > > defining a new key like you are doing currently for beeline-site.xml > > > > Is there a use-case which will not work in my suggestion above? May be > > I am missing something very obvious. Would appreciate if you could help me > > undestand. Thanks! > > Thejas Nair wrote: > Vihang, here are the requirements - > > * Beeline can be used in hosts that are not part of the hadoop cluster. > hive-site.xml might not be available there. Also, editing the large > hive-site.xml (if user is not in the cluster) is going to be complicated for > users. So we don't want to rely on hive-site.xml. > * Having the complete url as a config param in beeline-site.xml makes > easy to manage. You don't have to edit multiple parameters to add a new named > URL to it. Ambari provides full HS2 jdbc URL that is easy to copy and then > add to this file (when not managed by Ambari). > * If the host is part of a hadoop cluster, then beeline-site.xml can be > added by Ambari (or equivalent). However, there are client environment > related parameters that user might want to set in addition to that (SSL > truststore for example). In that case you don't want the user to modify > beeline-site.xml directly as it would get overwritten. So having a second > user override .xml file helps users to manage those settings seperately.
Thanks for the clarifying the details. Would appreciate if can you also clarify the below questions too. How do you handle the case where there are multiple HS2 instances on the cluster? Eg. one with Hive 2.3.x and one with Hive 3.0.0? Regarding the third point: Seems like the order of precendence is beeline-site.xml (first of $HOME/.beeline/beeline-site.xml, $HIVE_CONF_DIR/beeline-site.xml and /etc/hive/conf/beeline-site.xml) which gets overridden by beeline-hs2-connection.xml (first of $HOME/.beeline/beeline-site.xml, $HIVE_CONF_DIR/beeline-site.xml, /etc/hive/conf/beeline-site.xml) which gets merged into hive-site.xml if available. I agree with the usecase where user might want to override auto-generated file. Do you think it makes sense to have two different xml file names to do that? Isn't it confusing? Why not make it such that user-specific file in $HOME/.beeline/beeline-site.xml overrides the autogenerated $HIVE_CONF_DIR/beeline-site.xml or /etc/hive/conf/beeline-site.xml? if we do that we can get rid of one additional filename altogether. I am okay if you want to get rid of beeline-hs2-connection.xml if you follow the above approach. Please consider this a non-blocking suggestion so feel free to go ahead with the commit if you don't think the above is a good idea. I can create a follow-up JIRA separately and try to reconcile all these requirements by removing some of the redundant stuff. Thanks! - Vihang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66185/#review200215 ----------------------------------------------------------- On March 28, 2018, 8:27 p.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66185/ > ----------------------------------------------------------- > > (Updated March 28, 2018, 8:27 p.m.) > > > Review request for hive, Thejas Nair and Vihang Karajgaonkar. > > > Bugs: HIVE-18963 > https://issues.apache.org/jira/browse/HIVE-18963 > > > Repository: hive-git > > > Description > ------- > > https://issues.apache.org/jira/browse/HIVE-18963 > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 402fadddde > > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineConfFileParseException.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineHS2ConnectionFileParseException.java > acddf82a67 > > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParseException.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParser.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java > b769e8581f > > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java > f635b40633 > > beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java > 2801ebee09 > beeline/src/main/resources/BeeLine.properties 6fca953836 > > beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java > 1d17887417 > beeline/src/test/resources/test-hs2-named-connection-config.xml > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/BeelineWithHS2ConnectionFileTestBase.java > 3da31ad8a9 > jdbc/src/java/org/apache/hive/jdbc/Utils.java 6d7787da7d > > > Diff: https://reviews.apache.org/r/66185/diff/2/ > > > Testing > ------- > > > Thanks, > > Vaibhav Gumashta > >
