Re: [pgAdmin4][Patch]: Adding connection status in Query tool

On Wed, Dec 20, 2017 at 1:51 PM, Harshal Dhumal <
harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:

>
> On Wed, Dec 20, 2017 at 1:03 PM, Murtuza Zabuawala <
> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> On Wed, Dec 20, 2017 at 12:17 PM, Harshal Dhumal <
>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Tue, Dec 19, 2017 at 7:47 PM, Murtuza Zabuawala <
>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Dec 19, 2017 at 7:24 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> Interesting. A few thoughts:
>>>>>
>>>>> - The pulsating icon is very off-putting. I think we need to make it
>>>>> only flash a couple of times when we actually need to attract the attention
>>>>> of the user.
>>>>>
>>>> ​As per my discussion with Chethana, In his opinion user tends to
>>>> notice things that way more quickly.
>>>> Are you sure you wish to flash only couple of times on error?
>>>>
>>>>>
>>>>> - We shouldn't really use tooltips like this, as it may confuse folks
>>>>> with screen readers. Should we make the icon clickable (which should have a
>>>>> visual hint)? Maybe a drop-down status panel.
>>>>> ​
>>>>>
>>>>>
>>>> ​Sure let me check.
>>>> ​
>>>>
>>>>>
>>>>> - Do we need to poll separately for the status? Instead, why not
>>>>> update it whenever polling for results, or executing something?
>>>>>
>>>> ​Then user won't be able to know the current connection status prior to
>>>> query execution, the purpose of the feature is to make user aware of
>>>> current connection status even if there is no query running, As most user
>>>> tends to leave open their query tool window after their work it will be
>>>> useful when flask session gets expired and connection to server gets closed
>>>> after that.
>>>>
>>>
>>> - Status Ideal and Active can be piggy backed (when we are polling query
>>> result).
>>>
>> ​Checking for idle connection while polling the result is not possible.
>> because w
>> hen you press execute button
>> ​that
>>
>> ​means
>> are trying to execute some query then how do you check for idle status
>> when
>> ​there is already a
>> query
>> ​ running, you will always get Busy status :)
>> ​
>>
>
> We are using async connection. So to get result we're polling
> <http://initd.org/psycopg/docs/extensions.html#poll-constants> result
> every 0.1 seconds and wait for connection poll read
> <http://initd.org/psycopg/docs/extensions.html#poll-constants>.
> So in same poll http request you can alway call connection.get_transactio
> n_status().
> I just tried with select pg_sleep(1) and below is result.
>
> Note the last line it says 0 means transaction status was changed to Ideal
> when query execution was completed and connection poll status was 1 (
> POLL_READ
> <http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.POLL_READ>
> ).
>
> [image: Inline image 1]
>
>
> As I mentioned in my previous email the
purpose of the feature is to make user aware of current connection status
even
​​
if there is no query running
​ in query tool OR in easy words prior to query execution and after the
query execution
​ ​
and not in-between the query execution.

>
>>> - In case of connection closed then we are reconnecting seamlessly (as
>>> per your last comment on RM2475).
>>> So letting user know in advance that connection is closed does not
>>> provide any benefit.
>>>
>> I wrote that for desktop version, because we are automatically logging-in
>> user with each request if user is not authorized.​
>>
>>
>> Eventually user will look of any reconnect button/option in query tool
>>> to reconnect if we tell connection status in advance.
>>>
>> ​
>>>
>>>
>>
>>> - If flask session expires then (irrespective of connection status)
>>> status poll http request will be redirected to login page.
>>> So we won't get actual connection status in this case.
>>>
>>
>>>
>>>>
>>>>> Thanks!
>>>>>
>>>>> On Tue, Dec 19, 2017 at 11:42 AM, Murtuza Zabuawala <
>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> PFA patch to add the connection status
>>>>>> <http://initd.org/psycopg/docs/extensions.html#transaction-status-constants>
>>>>>> in query tool, this feature will allow user to check the database
>>>>>> connection status in query tool itself, it will also provide the detailed
>>>>>> status as a tooltip when user hovers on it, the most benefit of the feature
>>>>>> will be when user open query tool in new Browser Tab where Browser tree is
>>>>>> not visible to user, user can also configure the status polling time using
>>>>>> preference dialog.
>>>>>> RM#2475
>>>>>>
>>>>>> Apart from that I have also removed the
>>>>>> ..sqleditor/static/css/sqleditor.css reference from
>>>>>> ../datagrid/templates/datagrid/index.html file because we are
>>>>>> already bundling the "sqleditor.css" file in main "style.css" file.
>>>>>>
>>>>>>
>>>>>> Thanks to Chethana for his UI related inputs and to Surinder for
>>>>>> helping me on html alignment issues.
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Murtuza Zabuawala
>>>>>> EnterpriseDB: 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
>>>>>
>>>>
>>>>
>>>
>>
>