Re: [pgAdmin][RM4642] port should not be mandatory when a service is provided - Mailing list pgadmin-hackers
Hi,
On Fri, Sep 13, 2019 at 2:32 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi MurtuzaOn Fri, Sep 13, 2019 at 1:40 PM Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.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].
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.
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,On Fri, Sep 13, 2019 at 12:09 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:Looks good to me.On Fri, Sep 13, 2019 at 8:05 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Can someone please review this.On Mon, 9 Sep, 2019, 18:06 Akshay Joshi, <akshay.joshi@enterprisedb.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 & RegardsAkshay JoshiSr. Software ArchitectEnterpriseDB Software India Private LimitedMobile: +91 976-788-8246--Thanks and Regards,Aditya ToshniwalSoftware Engineer | EnterpriseDB India | Pune"Don't Complain about Heat, Plant a TREE"--Thanks & RegardsAkshay JoshiSr. Software ArchitectEnterpriseDB Software India Private LimitedMobile: +91 976-788-8246
pgadmin-hackers by date: