Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working in version 2.1

Thanks all, applied.

On Wed, May 30, 2018 at 5:06 PM, Joao De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)io> wrote:

> Hello Akshay,
>
> Thanks for taking a look. We did the following changes over your patch:
>
> - Changed the XPATH to CSS_SELECTOR, please look in
> pgadmin/feature_tests/query_tool_tests.py:660 and 667
> - Changed the other places in _query_tool_notify_statements to do not
> use xpath.
> - Moved the skip that previously was around the entire function to
> surround only the pg_notify call
>
> Thanks
> Victoria && Joao
> ​
>
> On Wed, May 30, 2018 at 1:52 AM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Joao
>>
>> On Tue, May 29, 2018 at 9:10 PM, Joao De Almeida Pereira <
>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>
>>> Hello Akshay,
>>>
>>> The code looks good, we do have some minor notes for next time:
>>>
>>> . To make the code easier to understand we should decide on 1 term and
>>> not have notifies and notifications to reference the same thing.
>>>
>> Fixed and used 'notifies' where it reference the same, but still at
>> some places 'notifications' is used because the new tab represents the
>> 'Notifications'.
>>
>>
>>> . It would be nice if we tried not to use XPATH to get the HTML
>>> components that we are testing on. Maybe try to use CSS classes. Selenium
>>> also has a driver.select_by_class_name which would work well in this
>>> case.
>>>
>> Tried to use both CSS_SELECTOR and CLASS_NAME, but it doesn't
>> support string comparison(contains method). I have googled for this and
>> most of the sites suggested to use XPATH where we can use contains()
>> method.
>>
>>
>>> The error that we are seeing in CI is related to pg_notify. This
>>> function was introduced in 9.0 and Greenplum is still not there yet. In
>>> order to solve this need to skip that part of the tests. There is an
>>> example in that same file using the function _test_explain_plan_feature
>>> to skip a tests depending on the Version.
>>>
>> OK. I have skip that part of the tests as per your suggestion. I have
>> rename the method "_test_explain_plan_feature" to
>> "_supported_server_version" which is more meaningful then the previous one.
>>
>> Attached is the modified patch. Please review it.
>>
>>
>>> Thanks
>>> Anthony && Joao
>>> ​
>>>
>>> On Mon, May 28, 2018 at 1:34 AM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Hakers,
>>>>
>>>> On my last patch feature tests were failed for GreenPlum5 database only
>>>> not for PostgreSQL. I have verified that on https://gpdb-dev.bosh.
>>>> pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/
>>>> run-tests/builds/93.
>>>>
>>>> Can someone from Pivotal team help me, as I don't have GreenPlum
>>>> database and don't know why it is failing.
>>>>
>>>> On Sat, May 26, 2018 at 5:47 PM, Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> As per suggestion by Dave and discussion with in the team, I have
>>>>> modified the logic again. Following are the modifications:
>>>>>
>>>>> - Instead of waiting for another query to execute on the session
>>>>> where 'LISTEN' command has been executed, we fetched the notify messages in
>>>>> the connection status polling logic. Doing this user will get the notify
>>>>> messages asap.
>>>>> - Instead of showing all the notifications in single alertify
>>>>> dialog, we call *alertify.info <http://alertify.info>('<msg>') *for
>>>>> individual notifications.
>>>>> - Created new tab 'Notifications' in Query Tool where all the
>>>>> notify messages will be recorded with the timestamp and payload.
>>>>>
>>>>> Please review the latest patch.
>>>>>
>>>>> On Tue, May 22, 2018 at 2:43 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Dave
>>>>>>>
>>>>>>> On Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <
>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Hackers,
>>>>>>>>>>
>>>>>>>>>> As per suggestion by Dave, I have modified the logic and now
>>>>>>>>>> notifications are popped up in alertify dialog(refer
>>>>>>>>>> Notify_Messages.png) as and when received on that session where
>>>>>>>>>> "LISTEN" is executed. Attached is the modified patch, please review it.
>>>>>>>>>>
>>>>>>>>>> To test this feature following steps need to perform:
>>>>>>>>>>
>>>>>>>>>> - Apply the patch.
>>>>>>>>>> - Run pgAdmin4
>>>>>>>>>> - Connect to any database server and open query tool.
>>>>>>>>>> - Execute 'LISTEN foo;' command.
>>>>>>>>>> - Open another query tool window and execute 'NOTIFY foo'.
>>>>>>>>>> (This is without payload).
>>>>>>>>>> - Execute 'select pg_notify('foo', 'Hello')' query (with
>>>>>>>>>> payload).
>>>>>>>>>> - Go to the query tool window from where 'LISTEN' was
>>>>>>>>>> executed and run any other query.
>>>>>>>>>>
>>>>>>>>>> I think there was a small misunderstanding here - I was
>>>>>>>>> suggesting that each notification be displayed in an Alertify notification,
>>>>>>>>> e.g. using alertify.message('A notification of FOO was received with
>>>>>>>>> payload '1234'...')
>>>>>>>>>
>>>>>>>>
>>>>>>> If there are too many notifications then it's annoying for
>>>>>>> user to popped up N number of alertify dialogs. Notification is
>>>>>>> only receives when any other query execute on the session where "LISTEN"
>>>>>>> command executes. So for example I have NOTIFY 10 times from different
>>>>>>> sessions and execute any other query on the session("LISTEN" one), 10
>>>>>>> alertify dialog will be popped up.
>>>>>>>
>>>>>>
>>>>>> Sure, but then a) it's the users choice to listen for something very
>>>>>> noisey, and b) if there are many notifications then the message box
>>>>>> dialogue will become huge.
>>>>>>
>>>>>> The other nice thing about using notifications is that they don't
>>>>>> require any acknowledgement; they show you the event happened, and then get
>>>>>> out of the way, allowing you to jump to the messages tab if needed.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> And it failed tests: https://gpdb-dev.bosh.
>>>>>>>> pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/
>>>>>>>> run-tests/builds/85 :-(
>>>>>>>>
>>>>>>>> Again it's timeout issue, not able to reproduce on my machine
>>>>>>> will look into it. Maybe will have to add webDriverWait.
>>>>>>>
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Mon, May 21, 2018 at 1:36 PM, Akshay Joshi <
>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Dave
>>>>>>>>>>>
>>>>>>>>>>> On Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <
>>>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Dave
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <
>>>>>>>>>>>>>> aemengo(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hey,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The code looks great! The tests all passed as well.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Agreed - however, unless you check the Messages panel, you're
>>>>>>>>>>>>>> not likely to see that a message was received.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Can we also show each message in an alertify panel?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> We need to change the design I guess, as we are currently
>>>>>>>>>>>>> send this as part of Messages. We will have to send this separately and
>>>>>>>>>>>>> show it in the alertify panel.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Yeah. Unfortunately I think notifications need to be more
>>>>>>>>>>>> "active" than the messages.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I am working on the above. Can we add one preferences
>>>>>>>>>>> setting to "ON/OFF" this alertify panel ?
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Dave Page
>>>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>
>>>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>>>
>>>>>>>>>>> *Sr. Software Architect *
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>>
>>>>>>>>>> *Sr. Software Architect *
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Dave Page
>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>
>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Dave Page
>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>> Twitter: @pgsnake
>>>>>>>>
>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Akshay Joshi*
>>>>>>>
>>>>>>> *Sr. Software Architect *
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Akshay Joshi*
>>>>>
>>>>> *Sr. Software Architect *
>>>>>
>>>>>
>>>>>
>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>
>>>
>>
>>
>> --
>> *Akshay Joshi*
>>
>> *Sr. Software Architect *
>>
>>
>>
>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company