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

On Thu, Apr 30, 2020 at 10:14 AM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> 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.
>
It's true that conn.connected() is misleading but we already get the
connection before checking conn.connected() with conn = manager.connection(
did=did, auto_reconnect=True).
So, if the database server is stopped, it will throw the error.

> 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.
>
It's a valid point, so I would suggest to show the warning/info at the
client side, no need to show the error message.

> 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"
>