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

Mobile: +91 976-788-8246
Attachment
Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working inversion 2.1
======================================================================ERROR: runTest (pgadmin.feature_tests.query_tool_tests.QueryToolFeatureTest)Query tool feature test----------------------------------------------------------------------Traceback (most recent call last):File "/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_tests/query_tool_tests.py", line 101, in runTestself._query_tool_notify_statements()File "/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_tests/query_tool_tests.py", line 643, in _query_tool_notify_statements'//div[contains(@class, "sql-editor-message") and 'File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py", line 169, in find_by_xpathlambda driver: driver.find_element_by_xpath(xpath)File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py", line 261, in wait_for_elementreturn self._wait_for("element to exist", element_if_it_exists)File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py", line 327, in _wait_for"Timed out waiting for " + waiting_for_messageFile "/root/.pyenv/versions/pgadmin/lib/python2.7/site-packages/selenium/webdriver/support/wait.py", line 80, in untilraise TimeoutException(message, screen, stacktrace)TimeoutException: Message: Timed out waiting for element to exist
Hi Hackers,Attached is the patch to capture the notification from psycopg2 and displayed it in "Messages" tab of query tool. Added feature test to cover this scenario.Refer Notification.png file to how it looks in "Messages" tab. Please review it.--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Hi Akshay,This patch is flaky; it doesn't always pass the tests in our pipeline.======================================================================ERROR: runTest (pgadmin.feature_tests.query_tool_tests.QueryToolFeatureTest)Query tool feature test----------------------------------------------------------------------Traceback (most recent call last):File "/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_tests/query_tool_tests.py", line 101, in runTestself._query_tool_notify_statements()File "/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_tests/query_tool_tests.py", line 643, in _query_tool_notify_statements'//div[contains(@class, "sql-editor-message") and 'File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py", line 169, in find_by_xpathlambda driver: driver.find_element_by_xpath(xpath)File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py", line 261, in wait_for_elementreturn self._wait_for("element to exist", element_if_it_exists)File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_utils/pgadmin_page.py", line 327, in _wait_for"Timed out waiting for " + waiting_for_messageFile "/root/.pyenv/versions/pgadmin/lib/python2.7/site-packages/selenium/webdriver/support/wait.py", line 80, in untilraise TimeoutException(message, screen, stacktrace)TimeoutException: Message: Timed out waiting for element to existAll the failures are related to query_tool_notify_statements. Please take another look.
Sincerely,Victoria & JoaoOn Tue, May 15, 2018 at 6:01 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi Hackers,Attached is the patch to capture the notification from psycopg2 and displayed it in "Messages" tab of query tool. Added feature test to cover this scenario.Refer Notification.png file to how it looks in "Messages" tab. Please review it.--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Hi JoaoOn Tue, 15 May 2018, 19:36 Joao De Almeida Pereira, <jdealmeidapereira@pivotal.io> wrote:Hi Akshay,This patch is flaky; it doesn't always pass the tests in our pipeline.============================================================ ========== ERROR: runTest (pgadmin.feature_tests.query_tool_tests. QueryToolFeatureTest) Query tool feature test------------------------------------------------------------ ---------- Traceback (most recent call last):File "/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_ tests/query_tool_tests.py", line 101, in runTest self._query_tool_notify_statements() File "/tmp/build/5988fb0a/pgadmin-repo/web/pgadmin/feature_ tests/query_tool_tests.py", line 643, in _query_tool_notify_statements '//div[contains(@class, "sql-editor-message") and 'File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_ utils/pgadmin_page.py", line 169, in find_by_xpath lambda driver: driver.find_element_by_xpath(xpath) File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_ utils/pgadmin_page.py", line 261, in wait_for_element return self._wait_for("element to exist", element_if_it_exists)File "/tmp/build/5988fb0a/pgadmin-repo/web/regression/feature_ utils/pgadmin_page.py", line 327, in _wait_for "Timed out waiting for " + waiting_for_messageFile "/root/.pyenv/versions/pgadmin/lib/python2.7/site- packages/selenium/webdriver/ support/wait.py", line 80, in until raise TimeoutException(message, screen, stacktrace)TimeoutException: Message: Timed out waiting for element to existAll the failures are related to query_tool_notify_statements. Please take another look.There is some serious problems of timeout issues. I have tested it couple of times and working fine. Will test tomorrow one more time. I have followed the code written in the same file. If those test cases have passed then this should also.
Sincerely,Victoria & JoaoHi Hackers,Attached is the patch to capture the notification from psycopg2 and displayed it in "Messages" tab of query tool. Added feature test to cover this scenario.Refer Notification.png file to how it looks in "Messages" tab. Please review it.--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246

Mobile: +91 976-788-8246
Attachment
Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working inversion 2.1
Hey,The code looks great! The tests all passed as well.
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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?
--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Mobile: +91 976-788-8246
Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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.
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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.
--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Mobile: +91 976-788-8246
Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working in version 2.1
- 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.
Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246

Mobile: +91 976-788-8246
Attachment
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.
On Mon, May 21, 2018 at 1:36 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246

Mobile: +91 976-788-8246
Attachment
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.
On Mon, May 21, 2018 at 1:36 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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'...')
On Mon, May 21, 2018 at 1:36 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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'...')
And it failed tests: https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/ pipelines/pgadmin-patch/jobs/ run-tests/builds/85 :-(
On Mon, May 21, 2018 at 1:36 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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

Mobile: +91 976-788-8246
Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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.
And it failed tests: https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pi pelines/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.
On Mon, May 21, 2018 at 1:36 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
- 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('<msg>') for individual notifications.
- Created new tab 'Notifications' in Query Tool where all the notify messages will be recorded with the timestamp and payload.
On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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/pi pelines/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.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Mobile: +91 976-788-8246
Attachment
Hi Hackers,As per suggestion by Dave and discussion with in the team, I have modified the logic again. Following are the modifications:Please review the latest patch.
- 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('<msg>') for individual notifications.
- Created new tab 'Notifications' in Query Tool where all the notify messages will be recorded with the timestamp and payload.
On Tue, May 22, 2018 at 2:43 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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/pi pelines/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.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246

Mobile: +91 976-788-8246
Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working inversion 2.1
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.
. 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.
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.
Thanks
Anthony && Joao
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.joshi@enterprisedb.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:Please review the latest patch.
- 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('<msg>') for individual notifications.
- Created new tab 'Notifications' in Query Tool where all the notify messages will be recorded with the timestamp and payload.
On Tue, May 22, 2018 at 2:43 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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.joshi@enterprisedb.com> wrote:Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
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
andnotifications
to reference the same thing.
. 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.
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.
Thanks
Anthony && JoaoHi 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.joshi@enterprisedb.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:Please review the latest patch.
- 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('<msg>') for individual notifications.
- Created new tab 'Notifications' in Query Tool where all the notify messages will be recorded with the timestamp and payload.
On Tue, May 22, 2018 at 2:43 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246

Mobile: +91 976-788-8246
Attachment
Re: [pgAdmin4][Patch] Feature #3204 Notify/Listen not working inversion 2.1
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
Hi JoaoOn Tue, May 29, 2018 at 9:10 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.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
andnotifications
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 && JoaoOn Mon, May 28, 2018 at 1:34 AM Akshay Joshi <akshay.joshi@enterprisedb.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.joshi@enterprisedb.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:Please review the latest patch.
- 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('<msg>') for individual notifications.
- Created new tab 'Notifications' in Query Tool where all the notify messages will be recorded with the timestamp and payload.
On Tue, May 22, 2018 at 2:43 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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.joshi@enterprisedb.com> wrote:Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Attachment
Hi JoaoOn Tue, May 29, 2018 at 9:10 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.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
andnotifications
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 && JoaoHi 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/pipeli nes/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.joshi@enterprisedb.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:Please review the latest patch.
- 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('<msg>') for individual notifications.
- Created new tab 'Notifications' in Query Tool where all the notify messages will be recorded with the timestamp and payload.
On Tue, May 22, 2018 at 2:43 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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/pi pelines/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.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
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
callThanks
Victoria && JoaoHi JoaoOn Tue, May 29, 2018 at 9:10 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.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
andnotifications
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 && JoaoHi 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.joshi@enterprisedb.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:Please review the latest patch.
- 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('<msg>') for individual notifications.
- Created new tab 'Notifications' in Query Tool where all the notify messages will be recorded with the timestamp and payload.
On Tue, May 22, 2018 at 2:43 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 10:01 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Tue, May 22, 2018 at 2:02 PM, Dave Page <dpage@pgadmin.org> wrote:On Tue, May 22, 2018 at 9:13 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Tue, May 22, 2018 at 7:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.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.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 4:56 PM, Dave Page <dpage@pgadmin.org> wrote:On Fri, May 18, 2018 at 12:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi DaveOn Fri, May 18, 2018 at 3:58 PM, Dave Page <dpage@pgadmin.org> wrote:HiOn Wed, May 16, 2018 at 2:51 PM, Anthony Emengo <aemengo@pivotal.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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +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 JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246--Akshay JoshiSr. Software ArchitectPhone: +91 20-3058-9517
Mobile: +91 976-788-8246
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company