Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201711141534.vAEFY5Nm029769@ip-10-146-233-104.ec2.internal>
Date: Tue, 14 Nov 2017 15:34:03 +0000
From: "Zoltan Borok-Nagy (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Tim Armstrong <tarmstrong@cloudera.com>,
        Philip Zeyliger <philip@cloudera.com>
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-2235=3A_Fix_current_db_when_shell_auto-reconnects=0A?=
In-Reply-To: <gerrit.1508855541000.I40dfa00ba0314d356fe8617446f516505c925e5e@gerrit.cloudera.org>
References: <gerrit.1508855541000.I40dfa00ba0314d356fe8617446f516505c925e5e@gerrit.cloudera.org>
Reply-To: boroknagyz@cloudera.com, impala-cr@cloudera.com, marcelk@gmail.com,
        tarmstrong@cloudera.com, philip@cloudera.com,
        reviews@impala.incubator.apache.org
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.14.2
Content-Type: multipart/alternative; boundary="XY3zrMt8vZY="; charset=UTF-8
archived-at: Tue, 14 Nov 2017 15:34:12 -0000

--XY3zrMt8vZY=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Zoltan Borok-Nagy has posted comments on this change=2E ( http://gerrit=2Ec=
loudera=2Eorg:8080/8368 )

Change subject: IMPALA-2235: Fix current db when=
 shell auto-reconnects
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E


Patch Set 4:

(3 comments)

Thanks for the review=2E

In additi=
on I also added a time=2Esleep(1) to the test case test_auto_reconnect beca=
use I noticed that it wasn't stable=2E
This sleep gives time for the Impala=
Shell to successfully start and connect to the Impala daemon before we rest=
art the cluster=2E

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8368/3//COMMIT_=
MSG
Commit Message:

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8368/3//COMMIT=
_MSG@9
PS3, Line 9: The ImpalaShell didn't issue the 'USE <current-db>' com=
mand after
> This sentence didn't parse for me=2E Specifically, I'm not sur=
e what "that" i
I rephrased the beginning of the commit=2E I hope it is eas=
ier to understand now=2E


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8368/3/t=
ests/custom_cluster/test_shell_interactive_reconnect=2Epy
File tests/custom=
_cluster/test_shell_interactive_reconnect=2Epy:

http://gerrit=2Ecloudera=
=2Eorg:8080/#/c/8368/3/tests/custom_cluster/test_shell_interactive_reconnec=
t=2Epy@36
PS3, Line 36: 
            :     cls=2Etempfile_name =3D tempfile=
=2Emktemp()
            :     move_shell_history(cls=2Etempfile_n
> Minor: =
I think this is just:
I only used NamedTemporaryFile because mktemp is mark=
ed as deprecated:
https://docs=2Epython=2Eorg/2/library/tempfile=2Ehtml#tem=
pfile=2Emktemp

tempfile=2Emkstemp() could have also been used, but it also=
 creates a file, and returns an open file descriptor for that=2E

Anyway, s=
ince it is in a test case, I don't see that mktemp() would cause a vulnerab=
ility issue, so now I use it for brevity and better readability=2E
Done=2E
=


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8368/3/tests/shell/util=2Epy
File=
 tests/shell/util=2Epy:

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8368/3/tes=
ts/shell/util=2Epy@121
PS3, Line 121: def restore_shell_history(filepath):
=
> mention that this is a no-op if filepath doesn't exist=2E
Done=2E


-- 
=
To view, visit http://gerrit=2Ecloudera=2Eorg:8080/8368
To unsubscribe, vis=
it http://gerrit=2Ecloudera=2Eorg:8080/settings

Gerrit-Project: Impala-ASF=

Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I40dfa=
00ba0314d356fe8617446f516505c925e5e
Gerrit-Change-Number: 8368
Gerrit-Patch=
Set: 4
Gerrit-Owner: Zoltan Borok-Nagy <boroknagyz@cloudera=2Ecom>
Gerrit-R=
eviewer: Philip Zeyliger <philip@cloudera=2Ecom>
Gerrit-Reviewer: Tim Armst=
rong <tarmstrong@cloudera=2Ecom>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokn=
agyz@cloudera=2Ecom>
Gerrit-Comment-Date: Tue, 14 Nov 2017 15:34:03 +0000
G=
errit-HasComments: Yes

--XY3zrMt8vZY=--