Re: PATCH/s: RM#1387 - Bad handling of missing connection database server

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: PATCH/s: RM#1387 - Bad handling of missing connection database server
Date: 2016-08-29 08:19:50
Message-ID: CAG7mmoxN7V9w3c48XziOM3gWyocWrC+OmmdczZ4F+c+SgZpXrQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks - committed!

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com>

*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi>

On Mon, Aug 29, 2016 at 10:40 AM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> Hi Ashesh,
>
> Patch is working for me, I did not find any major issues.
> But I have fixed some of the issues where alert was not displaying object
> name,
>
> PFA add-on patch to your previous patch.
>
> Please review.
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Thu, Aug 25, 2016 at 3:49 PM, Ashesh Vashi <
> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi Murtuza,
>>
>>
>> On Wed, Aug 24, 2016 at 3:17 PM, Murtuza Zabuawala <
>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Ashesh,
>>>
>>> Here are few scenario I tried which does not like working properly,
>>>
>>> 1) "properties" dialog do not open for any object while I'm able to open
>>> "create" dialog
>>>
>> Done.
>>
>>>
>>> 2) Click & connect the server, Now go to Statistics tab, you will see
>>> blank alert info
>>> [Console error] statistics.js:216 Uncaught ReferenceError: that is not
>>> defined
>>>
>> Done.
>>
>>>
>>> 3) Open "Create" dialog fill information, Stop DB & then click on Save
>>> button.
>>> [Console error] pgadmin.defaults.js:90 Uncaught TypeError:
>>> xhr.getResponseHeader is not a function
>>>
>> Done.
>>
>>>
>>> 4) If pgAdmin4 is running & database is connected, Now if database goes
>>> down (eg: stop db service),
>>> server stays as connected & there are no error from application when we
>>> traverse between tabs.
>>>
>> Due to point #2.
>>
>>>
>>> 5) If pgAdmin4 is running & one or more server is connected, Now if any
>>> database server goes down (eg: stop db service)
>>> Refresh entire page now click on expand server-group, loading/expanding
>>> server group goes infinite.
>>>
>> This is not regression of this patch.
>> I will share a separate patch for this.
>>
>> Please try the updated patches.
>>
>>>
>>> Thanks,
>>> Murtuza
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Wed, Aug 24, 2016 at 2:17 PM, Ashesh Vashi <
>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> On Wed, Aug 24, 2016 at 11:25 AM, Ashesh Vashi <
>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Team,
>>>>>
>>>>> On Sun, Aug 21, 2016 at 3:55 PM, Ashesh Vashi <
>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> On Aug 21, 2016 15:08, "Dave Page" <dpage(at)pgadmin(dot)org> wrote:
>>>>>> >
>>>>>> > Yeah, but shouldn't it be caught and handled, with a suitable retry
>>>>>> yes/no in the UI?
>>>>>> And, that realised me, I missed one case, on which I am already
>>>>>> working.
>>>>>>
>>>>> Please find the updated patches.
>>>>>
>>>> Thanks - Murtuza for point out the issue.
>>>>
>>>> The patches were not generated from the latest changes.
>>>> Please find the updated patches.
>>>>
>>>> --
>>>>
>>>> Thanks & Regards,
>>>>
>>>> Ashesh Vashi
>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>> <http://www.enterprisedb.com/>
>>>>
>>>>
>>>> *http://www.linkedin.com/in/asheshvashi*
>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>
>>>>>
>>>>> --
>>>>>
>>>>> Thanks & Regards,
>>>>>
>>>>> Ashesh Vashi
>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>> <http://www.enterprisedb.com/>
>>>>>
>>>>>
>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>
>>>>>> --
>>>>>> Thanks & Regards,
>>>>>>
>>>>>> Ashesh Vashi
>>>>>> >
>>>>>> >
>>>>>> > --
>>>>>> > Dave Page
>>>>>> > Blog: http://pgsnake.blogspot.com
>>>>>> > Twitter: @pgsnake
>>>>>> >
>>>>>> > EnterpriseDB UK:http://www.enterprisedb.com
>>>>>> > The Enterprise PostgreSQL Company
>>>>>> >
>>>>>> > On 20 Aug 2016, at 12:03, Ashesh Vashi <
>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>> >
>>>>>> >> On Fri, Aug 19, 2016 at 9:32 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>> wrote:
>>>>>> >>>
>>>>>> >>> Hi
>>>>>> >>>
>>>>>> >>> Can you re-base please? The nodes patch no longer applies.
>>>>>> >>>
>>>>>> >>> Ignoring that conflict though, the first test I did was to
>>>>>> restart the app server when connected to a database. I then (after it
>>>>>> restarted) to open the Schemas node in that database; nothing happened at
>>>>>> all in the client, but on the server I saw:
>>>>>> >>
>>>>>> >> This exception suggests that - pgAdmin 4 was not able to establish
>>>>>> the connection again during restarting it.
>>>>>> >>
>>>>>> >> --
>>>>>> >>
>>>>>> >> Thanks & Regards,
>>>>>> >>
>>>>>> >> Ashesh Vashi
>>>>>> >> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>> >>
>>>>>> >>
>>>>>> >> http://www.linkedin.com/in/asheshvashi
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> 2016-08-19 16:59:16,321: INFO werkzeug: 127.0.0.1 - -
>>>>>> [19/Aug/2016 16:59:16] "GET /browser/schema/nodes/1/1/219759/
>>>>>> HTTP/1.1" 500 -
>>>>>> >>> Traceback (most recent call last):
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 2000, in
>>>>>> __call__
>>>>>> >>> return self.wsgi_app(environ, start_response)
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1991, in
>>>>>> wsgi_app
>>>>>> >>> response = self.make_response(self.handle_exception(e))
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1567, in
>>>>>> handle_exception
>>>>>> >>> reraise(exc_type, exc_value, tb)
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1988, in
>>>>>> wsgi_app
>>>>>> >>> response = self.full_dispatch_request()
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1641, in
>>>>>> full_dispatch_request
>>>>>> >>> rv = self.handle_user_exception(e)
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1544, in
>>>>>> handle_user_exception
>>>>>> >>> reraise(exc_type, exc_value, tb)
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1639, in
>>>>>> full_dispatch_request
>>>>>> >>> rv = self.dispatch_request()
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/app.py", line 1625, in
>>>>>> dispatch_request
>>>>>> >>> return self.view_functions[rule.endpoint](**req.view_args)
>>>>>> >>> File "/Users/dpage/.virtualenvs/pga
>>>>>> dmin4/lib/python2.7/site-packages/flask/views.py", line 84, in view
>>>>>> >>> return self.dispatch_request(*args, **kwargs)
>>>>>> >>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py",
>>>>>> line 235, in dispatch_request
>>>>>> >>> return method(*args, **kwargs)
>>>>>> >>> File "/Users/dpage/git/pgadmin4/web
>>>>>> /pgadmin/browser/server_groups/servers/databases/schemas/__init__.py",
>>>>>> line 129, in wrap
>>>>>> >>> self.conn = self.manager.connection(did=kwargs['did'])
>>>>>> >>> File "/Users/dpage/git/pgadmin4/web
>>>>>> /pgadmin/utils/driver/psycopg2/__init__.py", line 1330, in connection
>>>>>> >>> raise Exception(msg_active_conn)
>>>>>> >>>
>>>>>> >>> I realise it could well be caused by the conflict when applying
>>>>>> the nodes patch, but just in case it's not... :-)
>>>>>> >>>
>>>>>> >>> On Fri, Aug 19, 2016 at 12:25 PM, Ashesh Vashi <
>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>> >>>>
>>>>>> >>>> Hi Dave/Team,
>>>>>> >>>>
>>>>>> >>>> I have attached three patches targeting specific area of work.
>>>>>> >>>>
>>>>>> >>>> -> backend_connection_status_management
>>>>>> >>>> * Takes care of the connection status in the psycopg2 driver.
>>>>>> And, when the connection is lost, it throws a exception with 503 http
>>>>>> status message, and connection lost information in it.
>>>>>> >>>> * Contains a change to allow to the flask application to
>>>>>> propagate the exceptions even in the release mode.
>>>>>> >>>> * Modification for utilising the existing password (while
>>>>>> reconnection, if not disconnected explicitly).
>>>>>> >>>> * Introduced a new ajax response message 'service_unavailable'
>>>>>> (http status code: 503), which suggests temporary service unavailable.
>>>>>> >>>>
>>>>>> >>>> -> manage_client_connect_lost
>>>>>> >>>> * It handles the connection lost for different operations at
>>>>>> client (browser) side.
>>>>>> >>>> * Using the events to handle this situation.
>>>>>> >>>>
>>>>>> >>>> -> remove_connection_status_check
>>>>>> >>>> * We're currently return precondition_required (HTTP status
>>>>>> code: 428) for the same, but - there is no consistent message/information
>>>>>> available to identify it reliably as the server connection lost error
>>>>>> (client side changes takes care of this situation, but - this patch is
>>>>>> recommended for the whole change to work properly). This patch removes the
>>>>>> connection status check from the individual nodes.
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> TODO:
>>>>>> >>>> * Browser tree - node expansion
>>>>>> >>>> - When there is an error occurred during expanding the aci-tree
>>>>>> node, it does not let the client handle the issue based on the status code,
>>>>>> and error message, as it does not expose/share the error details with us.
>>>>>> We will have to hack the aci-tree for the same.
>>>>>> >>>>
>>>>>> >>>> * Query Editor
>>>>>> >>>> - Slick grid changes are in progress, hence - I did not touch
>>>>>> the code for it, as it will lead to rework for either one of us (me, or
>>>>>> Murtuza). I will take care of it as soon as the slick grid changes will be
>>>>>> committed.
>>>>>> >>>>
>>>>>> >>>> --
>>>>>> >>>>
>>>>>> >>>> Thanks & Regards,
>>>>>> >>>>
>>>>>> >>>> Ashesh Vashi
>>>>>> >>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> http://www.linkedin.com/in/asheshvashi
>>>>>> >>>
>>>>>> >>>
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> --
>>>>>> >>> Dave Page
>>>>>> >>> Blog: http://pgsnake.blogspot.com
>>>>>> >>> Twitter: @pgsnake
>>>>>> >>>
>>>>>> >>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> >>> The Enterprise PostgreSQL Company
>>>>>> >>
>>>>>> >>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>>>> To make changes to your subscription:
>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>
>>>>
>>>
>>
>

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2016-08-29 10:00:36 Re: PATCH: SlickGrid integration in query tool (pgAdmin4)
Previous Message Akshay Joshi 2016-08-29 08:18:03 Re: [pgAdmin4][Patch]: RM#1238