Re: [pgAdmin][RM4642] port should not be mandatory when a service is provided

On Fri, Sep 13, 2019 at 3:01 PM Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> Hi,
>
> On Fri, Sep 13, 2019 at 2:32 PM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Murtuza
>>
>> On Fri, Sep 13, 2019 at 1:40 PM Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> We are making assumption here that all user's service file has port &
>>> username inside them.
>>> What if they are not present?
>>>
>>
>> 5432 is the default port and the system user is the default user to
>> connect.
>>
>
> Despite default values we had frontend validation on those fields from the
> beginning [Not specific to service file but in general].
>

Validations are still there until the user provides the service field.

[image: Screenshot 2019-09-13 at 3.18.56 PM.png] [image:
Screenshot 2019-09-13 at 3.19.15 PM.png]

>
>> If port and username are not provided in the service file then the user
>> can specify that in the dialog itself.
>>
>
> I know user can provide in the dialog itself but we are not validating
> that service file contains port and/or username information in it & still
> we allow to save the server, I think if we are removing the validation from
> frontend then we should verify it in the backend if the service file
> contains required fields or not and throw an error before saving the server.
>

Why we should read the service file and check for the validation, as I
mentioned if port and username is not provided in the service file then it
will try to connect to 5432 with username as the system user.

>
>
>>
>>> If I remember correctly those validation were left intentional when we
>>> added service field just to make sure that we connect to DB server from
>>> pgAdmin4 regardless of those information present in service file.
>>>
>>
>> I personally think that behaviour is wrong if my service file has the
>> correct port number then why should the user enter all those information
>> again, and what if the user provides the wrong port number from server
>> dialog, it trys to connect to the wrong port.
>>
>> One small suggestion such comments should come early as I have sent
>> the patch 4 days ago not after the patch has been committed.
>>
>
> Sorry I missed the earlier email, I only saw today morning with your
> reminder email.
>
>
> Regards,
> Murtuza
>
>
>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>>
>>> On Fri, Sep 13, 2019 at 12:09 PM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Looks good to me.
>>>>
>>>> On Fri, Sep 13, 2019 at 8:05 AM Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Can someone please review this.
>>>>>
>>>>> On Mon, 9 Sep, 2019, 18:06 Akshay Joshi, <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Hackers,
>>>>>>
>>>>>> Attached is the patch to fix the RM #4642 "port should not be
>>>>>> mandatory when a service is provided". *Cyril Jouve* has sent the
>>>>>> initial patch, but that only removes the validation on GUI, when we save
>>>>>> the server properties without port and username backend throws an error
>>>>>> because port and username is *NOT NULL* columns in the server table
>>>>>> in SQLite.
>>>>>>
>>>>>> I have removed the *NOT NULL *constraint from the port and username.
>>>>>> The maintenance database is required as our whole connection logic is based
>>>>>> on that, so I have added the NOT NULL constraint for that column.
>>>>>>
>>>>>> Please review it.
>>>>>>
>>>>>> --
>>>>>> *Thanks & Regards*
>>>>>> *Akshay Joshi*
>>>>>>
>>>>>> *Sr. Software Architect*
>>>>>> *EnterpriseDB Software India Private Limited*
>>>>>> *Mobile: +91 976-788-8246*
>>>>>>
>>>>>
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> Software Engineer | EnterpriseDB India | Pune
>>>> "Don't Complain about Heat, Plant a TREE"
>>>>
>>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*