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

Thanks, applied.

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.pivot
>>> alci.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.p
>>>>>>> ivotalci.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