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

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 adding one
letter variables and if we continue adding code to already convoluted
source files it is going to be very hard to achieve this objective.

Our recommendations for this change are:
- Name the variables with comprehensive names
- Extract functions where we can and try to wrap some tests around them
(ex: the javascript disabled functions)
- We really need to find a better pattern than templated Javascript to pass
information from the backend to the frontend
- When changing a piece of code, if we see code that is confusing or that
is hard to read, we should refactor instead of adding to the pattern.

Thanks
Victoria & Joao

On Tue, Apr 24, 2018 at 10:13 AM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> 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 <aemengo(at)pivotal(dot)io>
> wrote:
>
>> 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 <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Mon, Apr 23, 2018 at 1:30 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Apr 19, 2018 at 6:56 PM, Anthony Emengo <aemengo(at)pivotal(dot)io>
>>>> 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 difficult, maybe impossible to do without
>>>> external infrastructure and config).
>>>>
>>>
>>> Agreed, it's been difficult to write test case to test the complete
>>> feature.
>>>
>>>>
>>>>
>>>>>
>>>>> Anthony and Victoria
>>>>>
>>>>> On Thu, Apr 19, 2018 at 1:48 AM, Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> 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 implementation supports user name
>>>>>> /password and private/public key combination with Passphrase to crate SSH
>>>>>> Tunnel. I have added regression test case to add server using SSH Tunnel
>>>>>> options.
>>>>>>
>>>>>> The given python package(https://pypi.org/project/sshtunnel/) support
>>>>>> Python version *2.7, 3.4+*.
>>>>>> It uses Paramiko (Python implementation of SSHv2 protocol) which
>>>>>> actually drops support for Python 2.6. So I have added
>>>>>> *SUPPORT_SSH_TUNNEL* parameter in config.py which checks the python
>>>>>> version and set the flag accordingly. In case of Python 2.6, 3.0, 3.1, 3.2
>>>>>> and 3.3 control on the "SSH Tunnel" tab of server dialog will be disabled.
>>>>>>
>>>>>> Please review it, and if looks good please commit the code.
>>>>>>
>>>>>> --
>>>>>> *Akshay Joshi*
>>>>>>
>>>>>> *Sr. Software Architect *
>>>>>>
>>>>>>
>>>>>>
>>>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>>>>> 976-788-8246 <+91%2097678%2088246>*
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>> 976-788-8246 <+91%2097678%2088246>*
>>>
>>
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>