On Fri, Mar 24, 2017 at 7:19 PM, Atira Odhner <aodhner@pivotal.io> wrote:
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.
I understand your concern about the Exception object.
But - what was the need to change the existing code, where it was working.
i.e.
@@ -783,7 +785,7 @@ class TableView(PGChildNodeView, DataTypeReader, VacuumSettings):
On Fri, Mar 24, 2017 at 5:07 PM, Dave Page <dpage@pgadmin.org> wrote:
Ashesh, can you review/commit this please?
On Thu, Mar 23, 2017 at 3:49 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.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'.
