> On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote: > > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java, > > line 88 > > <https://reviews.apache.org/r/51593/diff/2/?file=1492642#file1492642line88> > > > > I might prefer not to expose the file location on the interface, if it > > is not absolutely required. > > > > What do you think of using instead of this a new public method: > > > > public boolean isConfigExists()?
Good idea. Added a new interface method configExists() instead of getFileLocation. > On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote: > > beeline/src/java/org/apache/hive/beeline/BeeLine.java, lines 981-982 > > <https://reviews.apache.org/r/51593/diff/2/?file=1492639#file1492639line981> > > > > Public + VisibleForTesting This is a marker annotation for documetation purposes only. The method needs to be more visible than required only for test coverage. I have added a comment saying the same > On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote: > > beeline/src/java/org/apache/hive/beeline/BeeLine.java, lines 976-977 > > <https://reviews.apache.org/r/51593/diff/2/?file=1492639#file1492639line976> > > > > Public + VisibleForTesting This is a marker annotation for documetation purposes only. The method needs to be more visible than required only for test coverage. I have added a comment saying the same > On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote: > > beeline/src/java/org/apache/hive/beeline/BeeLine.java, lines 955-957 > > <https://reviews.apache.org/r/51593/diff/2/?file=1492639#file1492639line955> > > > > I am not sure about this, and this is more like a requirement question, > > than a coding question: > > - We might like to use the hive-site.xml based connection data, even > > there is no user specific connection data. > > > > For example: > > - We have an unsecured cluster, so we do not mind, if everyone uses the > > same username, password using hive. > > > > This might be a valid usecase. We still can chose not to support this, > > since this is a bad practice :) > > > > So this is just a question, nothing more! I had thought about this particular case. The issue is this change needs to be backwards compatible which means that previous methods of connecting using url should work. That means Beeline should know using some flag to use hive-site.xml or not. The presence of this file indicates that hive-site.xml is used or not. I had initially added an explicit flag for the same in user conf file, but seemed redundant to me. So if user would like to use only hive-site.xml for connection info, the can provide an empty file and it will serve as a flag to look for hive-site.xml. This may be true in a secure, kerberos enabled cluster where hive-site.xml has all the information that we need to connect. But there is a bug where beeline still asks for username, password even in kerberos enabled cluster, so currently, even in that case there needs to be some user-specific information which needs to be provided. I am planning to fix that case in a separate change. For all the other changes, there needs t o be a non-empty user-specific file which atleast provides user/password since hive-site.xml doesn't have that information. - Vihang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51593/#review148007 ----------------------------------------------------------- On Sept. 7, 2016, 6:22 p.m., Vihang Karajgaonkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51593/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2016, 6:22 p.m.) > > > Review request for hive, Mohit Sabharwal, Sergio Pena, and Szehon Ho. > > > Bugs: HIVE-14063 > https://issues.apache.org/jira/browse/HIVE-14063 > > > Repository: hive-git > > > Description > ------- > > This change adds a new optional configuration file for Beeline. If this file > is present at predefined locations, Beeline will attempt to create the > connection url using the hive-site.xml found in classpath and another > user-specific configuration file. Beeline then connects automatically using > the url generated based on these configuration files. The main objective of > the change is to provide user another way to connect to the HiveServer2 > without providing the connection url everytime. The configuration file uses > hadoop xml format so that we can support encryption/obfuscation using hadoop > credential manager API in the future. > > Properties in the user-specific configuration file override the properties > derived from hive-site.xml. > > Tested using newly added unit tests and itests in various Hiveserver2 > configurations. > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java > 8e65e3987398531cce5c65c383762cf49a52c578 > beeline/src/java/org/apache/hive/beeline/Commands.java > 2f3ec134098dfa3767bab9545438d1f38f11697c > > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineHS2ConnectionFileParseException.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/HiveSiteHS2ConnectionFileParser.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java > PRE-CREATION > > beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java > PRE-CREATION > beeline/src/test/resources/hive-site.xml > 5f310d68245275ac9dc24df45579784019eea332 > beeline/src/test/resources/test-hs2-conn-conf-kerberos-http.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-conn-conf-kerberos-nossl.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-conn-conf-kerberos-ssl.xml PRE-CREATION > beeline/src/test/resources/test-hs2-connection-conf-list.xml PRE-CREATION > beeline/src/test/resources/test-hs2-connection-config-noauth.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-connection-multi-conf-list.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-connection-zookeeper-config.xml > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/TestBeelineConnectionUsingHiveSite.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/TestBeelineWithHS2ConnectionFile.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/TestBeelineWithUserHs2ConnectionFile.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/51593/diff/ > > > Testing > ------- > > > Thanks, > > Vihang Karajgaonkar > >
