> On July 1, 2016, 9:58 p.m., Vihang Karajgaonkar wrote: > > Thanks for the changes. Can you please test your code by setting > > USE_DEPRECATED_CLI=false. > > Peter Vary wrote: > Ok. Maybe something is not yet clear for me :) > Could you please explain? > > My understanding of the command prompt clients: > - CLI - Deprecated - uses org.apache.hadoop.hive.cli.CliDriver class, > when the USE_DEPRECATED_CLI is set, then it uses this class to start - no > developement is planned > - Beeline Cli - The new one, and this should be used, and developed > further, and could start in two modes: > -- Beeline mode - org.apache.hive.beeline.BeeLine class will start in > this mode (beeLine.isBeeLine()==true) > -- Compatibility mode - org.apache.hive.beeline.cli.HiveCli class will > start in this mode (beeLine.isBeeLine()==false) > > Depending on the connection URL, the Beeline Cli could start an embedded > HS2 server - This is independent of the compatibility mode settings, and > dependent only on the connection url. I have learned this since your, and > Szehon Ho's review, so I have to change my comments at least :) > > Assuming the above is correct, I think we should not change the behavior > of the CLI, and the Beeline in Compatiblity mode. > I think we should only use the new parameter in Beeline/Beeline mode, > like it is happened with the showHeader command line option. > > If something is wrong with the above assumptions, or you think we should > use the new parameter in another modes as well, just say so, and I will look > at it > > Thanks, and sorry, but still learning the code, and the hive specific > practices
Your understanding is correct :). I just thought of checking if you have tested it all the modes above. In cli mode it should honour HiveConf.ConfVars.CLIPRINTCURRENTDB and in beeline mode the --showDbInPrompt argument.. > On July 1, 2016, 9:58 p.m., Vihang Karajgaonkar wrote: > > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 1446 > > <https://reviews.apache.org/r/49498/diff/1/?file=1434737#file1434737line1446> > > > > Is there a reason to remove previous check for > > HiveConf.ConfVars.CLIPRINTCURRENTDB? Looks like this method is getting > > called from getPromptForCli() and if the beeline option for db is false, it > > will not set db name in the cli prompt too irrespective of the value of > > HiveConf.ConfVars.CLIPRINTCURRENTDB. btw, you may want to look at > > HIVE-14151 if you finding trouble while using beeline as cli > > Peter Vary wrote: > I moved the HiveConf.ConfVars.CLIPRINTCURRENTDB check inside the > getShowDbInPrompt() method, to match the outline of the getShowHeader() > parameter. > > Thanks for the pointer, it helped a lot in testing and understanding :) Got it thanks. - Vihang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49498/#review140391 ----------------------------------------------------------- On July 1, 2016, 3:08 p.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49498/ > ----------------------------------------------------------- > > (Updated July 1, 2016, 3:08 p.m.) > > > Review request for hive, Sergio Pena, Szehon Ho, and Vihang Karajgaonkar. > > > Bugs: HIVE-14123 > https://issues.apache.org/jira/browse/HIVE-14123 > > > Repository: hive-git > > > Description > ------- > > There are several jira issues complaining that, the Beeline does not respect > hive.cli.print.current.db. > This is partially true, since in embedded mode, it uses the > hive.cli.print.current.db to change the prompt, since HIVE-10511. > In remote mode, I think this function should use a beeline command line > option instead, like for the showHeader option emphasizing, that this is a > client side option. > > The patch contains: > - New configuration option > - Changing the help text > - Updating command hooks, to run in remote mode as well > - Adding new hooks, for connect and go sqllite commands > - Generalize database connection refresh command > - Changing prompt > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 66185f6 > beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 5aaa385 > beeline/src/java/org/apache/hive/beeline/ClientCommandHookFactory.java > c4d97bc > beeline/src/java/org/apache/hive/beeline/Commands.java 3a204c0 > beeline/src/java/org/apache/hive/beeline/ReflectiveCommandHandler.java > 3b863ae > beeline/src/main/resources/BeeLine.properties 7500df9 > beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java ce1f538 > beeline/src/test/org/apache/hive/beeline/TestClientCommandHookFactory.java > c86de0a > > Diff: https://reviews.apache.org/r/49498/diff/ > > > Testing > ------- > > Unit tests for the hooks, and the configuration option > > Manual test in remote, and embedded mode > > > Thanks, > > Peter Vary > >
