Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query
Date: 2018-02-28 11:49:19
Message-ID: CAFOhELfXSuAK3YpXRA=mJXgUjuwsi4s=e80wEm70YsHfuMNyMA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Mon, Feb 26, 2018 at 10:02 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Argh, I ran some tests, but didn't spot any lost messages in the tests I
> ran. I'll revert the patch.
>
> Khushboo;
>
> Please look at the following:
>
> - Fix the patch so it doesn't drop messages.
>
Fixed.
By default, the notice attribute of the connection object of psycopg 2 only
stores 50 notices. Once it reaches to 50 it starts from 1 again.
To fix this I have changed the notice attribute from list to deque to
append more messages. Currently I have kept the maximum limit at a time of
the notice attribute is 100000 (in a single poll).

> - Add regression tests to make sure it doesn't break in the future. This
> may require creating one or more functions the spew out a whole lot of
> notices, and then running a couple of queries and checking the output.
>
Added. With this regression test, the current code is failing which has
been taken care in this patch.

> - Check the messages panel on the history tab. I just noticed it seems to
> only be showing an even smaller subset of the messages.
>
Tested and no issues found.

>
>
Thanks.
>
> On Mon, Feb 26, 2018 at 4:23 PM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Sent bit early,
>>
>> You can run 'VACUUM FULL VERBOSE' in query tool and verify the populated
>> messages (pgAdmin3 vs. pgAdmin4).
>>
>>
>> On Mon, Feb 26, 2018 at 9:48 PM, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Khushboo/Dave,
>>>
>>> With given commit, I'm again seeing the issue raised in
>>> https://redmine.postgresql.org/issues/1523 :(
>>>
>>>
>>>
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> On Mon, Feb 26, 2018 at 7:49 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Ensure we pick up the messages from the current query and not a
>>>> previous one. Fixes #3094
>>>>
>>>> Branch
>>>> ------
>>>> master
>>>>
>>>> Details
>>>> -------
>>>> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdif
>>>> f;h=08b3ccc01a4d57e8ea3657f8882a53dcd1b99386
>>>> Author: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
>>>>
>>>> Modified Files
>>>> --------------
>>>> web/pgadmin/utils/driver/abstract.py | 1 +
>>>> web/pgadmin/utils/driver/psycopg2/__init__.py | 64
>>>> +++++++++------------------
>>>> 2 files changed, 21 insertions(+), 44 deletions(-)
>>>>
>>>>
>>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
RM_3094_ver1.patch text/x-patch 10.7 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Neethu Mariya Joy 2018-02-28 12:59:16 Feature #3061
Previous Message Harshal Dhumal 2018-02-28 11:24:54 Re: RM3079 fix for wrong sql datetime/time related datatypes