On Fri, Mar 24, 2017 at 7:19 PM, Atira Odhner <aodh...@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):* * 25 status, result = self.conn.execute_dict(sql)* * 26 •* * 27 if not status:* * 28 - return internal_server_error(errormsg=result)* * 29 + raise InternalServerError(result)* * 30 •* * 31 for fk in result['rows']:* * 32 •* In above code (and, similar), why do we need that change at all? -- Thanks, Ashesh > > Tira & Joao > > On Fri, Mar 24, 2017 at 9:15 AM, Ashesh Vashi < > ashesh.va...@enterprisedb.com> wrote: > >> On Fri, Mar 24, 2017 at 5:07 PM, Dave Page <dp...@pgadmin.org> wrote: >> >>> Ashesh, can you review/commit this please? >>> >>> On Thu, Mar 23, 2017 at 3:49 PM, Joao Pedro De Almeida Pereira >>> <jdealmeidapere...@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'. >> >> Can you please elaborate about that change? >> >> >> -- Thanks, Ashesh >> >> > >>> > >>> > Thanks >>> > Joao & Sarah >>> > >>> > >>> > -- >>> > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.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 >>> >> >> >
