Re: [patch] Raise InternalServerError while retrieving table DDL

When we were working on the DDL story, we found that some methods were
returning the internal_server_error json, but the code that called those
methods was not expecting that type of object. So, instead of returning
that json to the user, the code would try to treat the json as a different
type of object which often resulted in a different internal server error
than the one that was intended.

We decided to raise the error instead so that there would be no risk of the
code interacting with the error object in an unexpected way, since it will
raise up and skip over that code. Then, we added a handler for the error
which returns the same type of internal_server_error which was intended
originally. This ensures that the user sees the proper error messages.

Tira & Joao

On Fri, Mar 24, 2017 at 9:15 AM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> On Fri, Mar 24, 2017 at 5:07 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Ashesh, can you review/commit this please?
>>
>> On Thu, Mar 23, 2017 at 3:49 PM, Joao Pedro De Almeida Pereira
>> <jdealmeidapereira(at)pivotal(dot)io> wrote:
>> > Hi Hackers,
>> >
>> > While doing the DDL patch we found out that the code was not working
>> > properly when errors happened during SQL execution:
>> >
>> > One example of this can be found in the file:
>> >
>> > web/pgadmin/browser/server_groups/servers/databases/schemas/
>> tables/__init__.py
>> >
>> > The function '_formatter' that returns internal_server_error when an
>> error
>> > occur executing SQL
>> > Nevertheless the 'sql' function uses the output of '_formatter' during
>> the
>> > execution without checking it.
>> >
>> > To solve this issue we raise an InternalServerError exception that we
>> catch
>> > in the 'sql' function instead of returning an error message.
>>
> Hi Joa & Sarah,
>
> I am not against putting the try..except block.
>
> We're already have out own version of 'internal_server_error', which will
> return value in JSON format.
> But - I did not understand the reason to change them with
> 'werkzeug.exceptions.InternalServerError'.
>
> Can you please elaborate about that change?
>
>
> -- Thanks, Ashesh
>
> >
>> >
>> > Thanks
>> > Joao & Sarah
>> >
>> >
>> > --
>> > 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
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>