MIME-Version: 1.0
In-Reply-To: 
 <CANxoLDdeG7k45nD=_gM_9qJeASXfPyert1yc7aZoPToVDcHnOQ@mail.gmail.com>
References: 
 <CANxoLDdeG7k45nD=_gM_9qJeASXfPyert1yc7aZoPToVDcHnOQ@mail.gmail.com>
From: Dave Page <dpage@pgadmin.org>
Date: Mon, 18 Jun 2018 18:09:04 +0100
Message-ID: 
 <CA+OCxozvzpGn2QQnutxLwAhV8Ag1Aze-jGn9EtuEQiuGS89YYw@mail.gmail.com>
Subject: Re: [pgadmin4][Patch]: Feature #3388 Allow to set the connection
 timeout on the server properties dialog
To: Akshay Joshi <akshay.joshi@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="0000000000007142fa056eed9f2f"
Precedence: bulk

--0000000000007142fa056eed9f2f
Content-Type: text/plain; charset="UTF-8"

Hi

On Mon, Jun 18, 2018 at 12:18 PM, Akshay Joshi <
akshay.joshi@enterprisedb.com> wrote:

> Hi Hackers,
>
> Attached is the patch for Feature #3388 Allow to set the connection
> timeout on the server properties dialog. Please review it.
>

I took a quick look at this and had some questions/comments:

- Doc updates seem to be missing.

- The timeout should be passed through to external utilities like
pg_dump/pg_restore.

- Can/should the timeout be extended to the SSH tunnels?

- Any refactoring opportunities in the code you're touching?

Thanks.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--0000000000007142fa056eed9f2f
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi<div class=3D"gmail_extra"><br><div class=3D"gmail_quote=
">On Mon, Jun 18, 2018 at 12:18 PM, Akshay Joshi <span dir=3D"ltr">&lt;<a h=
ref=3D"mailto:akshay.joshi@enterprisedb.com" target=3D"_blank">akshay.joshi=
@enterprisedb.com</a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote=
" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><=
div dir=3D"ltr"><span style=3D"font-size:12.8px;text-decoration-style:initi=
al;text-decoration-color:initial;float:none;display:inline">Hi Hackers,</sp=
an><div style=3D"font-size:12.8px;text-decoration-style:initial;text-decora=
tion-color:initial"><br></div><div style=3D"text-decoration-style:initial;t=
ext-decoration-color:initial"><span style=3D"font-size:12.8px">Attached is =
the patch for Feature #3388 Allow to set the connection timeout on the serv=
er properties <span id=3D"m_7363022926692653503:2ce.4">dialog</span>. Pleas=
e review it.</span></div></div></blockquote><div><br></div><div>I took a qu=
ick look at this and had some questions/comments:</div><div><br></div><div>=
- Doc updates seem to be missing.</div><div><br></div><div>- The timeout sh=
ould be passed through to external utilities like pg_dump/pg_restore.</div>=
<div><br></div><div>- Can/should the timeout be extended to the SSH tunnels=
?=C2=A0</div><div><br></div><div>- Any refactoring opportunities in the cod=
e you&#39;re touching?</div><div><br></div><div>Thanks.</div></div><div><br=
></div>-- <br><div class=3D"gmail_signature" data-smartmail=3D"gmail_signat=
ure">Dave Page<br>Blog: <a href=3D"http://pgsnake.blogspot.com" target=3D"_=
blank">http://pgsnake.blogspot.com</a><br>Twitter: @pgsnake<br><br>Enterpri=
seDB UK: <a href=3D"http://www.enterprisedb.com" target=3D"_blank">http://w=
ww.enterprisedb.com</a><br>The Enterprise PostgreSQL Company<br></div>
</div></div>

--0000000000007142fa056eed9f2f--