Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-05-04 Thread Dave Page
On Thu, May 3, 2018 at 10:06 AM, Akshay Joshi wrote: > Hi > > On Thu, May 3, 2018 at 2:20 PM, Dave Page wrote: > >> >> >> On Thu, May 3, 2018 at 6:58 AM, Akshay Joshi < >> akshay.jo...@enterprisedb.com> wrote: >> >>> Hi >>> >>> On Wed, May 2, 2018 at 9:05 PM, Victoria Henry >>> wrote: >>>

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-05-03 Thread Akshay Joshi
Hi On Thu, May 3, 2018 at 2:20 PM, Dave Page wrote: > > > On Thu, May 3, 2018 at 6:58 AM, Akshay Joshi < > akshay.jo...@enterprisedb.com> wrote: > >> Hi >> >> On Wed, May 2, 2018 at 9:05 PM, Victoria Henry wrote: >> >>> Hi Akshay, >>> >>> Thanks for sending this updated patch. The linter and t

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-05-03 Thread Dave Page
On Thu, May 3, 2018 at 6:58 AM, Akshay Joshi wrote: > Hi > > On Wed, May 2, 2018 at 9:05 PM, Victoria Henry wrote: > >> Hi Akshay, >> >> Thanks for sending this updated patch. The linter and tests are all >> passing. >> >>> - utils/driver/psycopg2/server_manager.py - Do we have Unit Test

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-05-02 Thread Akshay Joshi
Hi On Wed, May 2, 2018 at 9:05 PM, Victoria Henry wrote: > Hi Akshay, > > Thanks for sending this updated patch. The linter and tests are all > passing. > >> - utils/driver/psycopg2/server_manager.py >>> - Do we have Unit Tests around this? >> >> No. > > > In our opinion, server_manager.py a

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-05-02 Thread Victoria Henry
Hi Akshay, Thanks for sending this updated patch. The linter and tests are all passing. > - utils/driver/psycopg2/server_manager.py >> - Do we have Unit Tests around this? > > No. In our opinion, server_manager.py and connection.py should have tests. Are you finding it difficult to add tes

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-26 Thread Joao De Almeida Pereira
Hi Akshay, Some suggestions: - browser/server_groups/servers/__init__py This file could have been split into separate functionalities. There is a chunk of changes for connect, why not move that out? Same thing for create. Do we really need to have full integrationy tests that do a HTTP request a

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-24 Thread Akshay Joshi
Hi Joao On Tue, Apr 24, 2018 at 10:04 PM, Joao De Almeida Pereira wrote: > Hi Akshay, > > After looking through the patch we found some one letter variable names > and this is a regression on what we have been trying to accomplish in the > last year. > > An objective that we have for pgAdmin sou

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-24 Thread Joao De Almeida Pereira
Hi Akshay, After looking through the patch we found some one letter variable names and this is a regression on what we have been trying to accomplish in the last year. An objective that we have for pgAdmin source code is to increase the testability of it and make it more readable. If we keep on a

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-24 Thread Akshay Joshi
Hi Hackers As per suggestion by Dave, I have moved "Advanced" tab at the last for Server dialog. Attached is the modified patch. On Mon, Apr 23, 2018 at 7:32 PM, Anthony Emengo wrote: > For what it is worth, I manually verified that the feature worked, as well > as looked through the code. > >

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-23 Thread Anthony Emengo
For what it is worth, I manually verified that the feature worked, as well as looked through the code. I'd like to see end-to-end testing for regression sake, but it's hard to so at this moment. - Anthony and Joao. On Mon, Apr 23, 2018 at 5:09 AM, Akshay Joshi wrote: > > > On Mon, Apr 23, 2018

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-23 Thread Akshay Joshi
On Mon, Apr 23, 2018 at 1:30 PM, Dave Page wrote: > Hi > > On Thu, Apr 19, 2018 at 6:56 PM, Anthony Emengo > wrote: > >> Hey Akshay >> >> This patch passed our test pipelines. >> > > Did you test the feature and//or review the code and tests? Passing the > tests is great, *if* the whole feature

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-23 Thread Dave Page
Hi On Thu, Apr 19, 2018 at 6:56 PM, Anthony Emengo wrote: > Hey Akshay > > This patch passed our test pipelines. > Did you test the feature and//or review the code and tests? Passing the tests is great, *if* the whole feature is covered (and the nature of this patch will make that quite difficu

Re: [pgAdmin4][Patch] Feature #1447 SSH Tunnel

2018-04-19 Thread Anthony Emengo
Hey Akshay This patch passed our test pipelines. Anthony and Victoria On Thu, Apr 19, 2018 at 1:48 AM, Akshay Joshi wrote: > Hi Hackers > > I have implemented the SSH Tunnel support using https://pypi.org/ > project/sshtunnel/ python package. Added "SSH Tunnel" Tab in server > dialog. This imp