Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Satish V <satish(dot)v(at)enterprisedb(dot)com>
Cc: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Date: 2020-04-30 04:44:07
Message-ID: CAM9w-_=jOvu9wa6yccQHpomZ87wsiXczqw9+y60o82Oj4VCoQA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

On Thu, Apr 30, 2020 at 9:41 AM Satish V <satish(dot)v(at)enterprisedb(dot)com> wrote:

> Hi Kushboo,
>
> Thanks for the update. I will check the same and make appropriate changes.
>
> Thanks,
> Sathish
>
> On Thu, Apr 30, 2020 at 9:20 AM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi Satish,
>>
>> As per the RM, the fix is supposed to be at the front-end but it seems
>> difficult at the moment as on the selection of the database, we connect it
>> and at the same time the context menu is being called.
>> As you have tried to fix at the backend, some of the review comments are
>> below.
>>
>> 1. If the database is already connected, no need to call conn.connect
>> again.
>>
>>
>> info_already_connected = conn.connected()
>>
>> status, errmsg = conn.connect()
>>
>> I've noticed conn.connected() is misleading sometimes. Let's say if the
PG server is stopped and no query is fired from pgadmin after that,
then conn.connected()
will still give True. It is updated only when a query is fired to the PG
server. I would suggest let it connect again as there is no harm and this
function is very important. We don't want to mess it up for the sake of a
message.

> 2. If you want to raise an error at the client side, use the appropriate function or status (check file ajax.py) at the server side and send the appropriate response.
>>
>> Below code needs to be changed.
>>
>> if(res.data.info_already_connected){
>>
>> Alertify.error(res.info);
>>
>> } else {
>>
>> Alertify.success(res.info);
>>
>> }
>>
>> The problem is test cases. Almost all the test cases have checked for
response "Database connected.". If the function is changed at the server
side then all the test cases (around 85+) would have to be changed to
handle this.

> Thanks,
>>
>> Khushboo
>>
>>
>>
>> On Wed, Apr 29, 2020 at 8:20 PM Satish V <satish(dot)v(at)enterprisedb(dot)com>
>> wrote:
>>
>>> Hi Hackers,
>>>
>>> In the patch attached, we are gracefully informing the end user, using
>>> an alert message, that the database is already connected when they click
>>> "Connect Database..." after right clicking on a disconnected database.
>>>
>>> As this problem deals with racing conditions, it is highly complex to
>>> show the "Disconnect database" option in the menu upon right click. So we
>>> are alerting the end user with the information of "Database already
>>> connected".
>>>
>>> Thanks,
>>> Sathish V
>>>
>>

--
Thanks and Regards,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2020-04-30 05:18:39 Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu
Previous Message Satish V 2020-04-30 04:11:20 Re: RM3694-If database is already connected and click on database then connect database should not displayed in Menu