public inbox for [email protected]  
help / color / mirror / Atom feed
From: Akshay Joshi <[email protected]>
To: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch] Add formatted error message support in psycopg2
Date: Thu, 31 Mar 2016 12:37:40 +0530
Message-ID: <CANxoLDck1+6BXZvjcQ7bSfGGsnESV74-fWpkuwUsJm8Fuh9GwA@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDfRgTKK7rJPR14R8zNnmq3Uu1p37KbMYTba8coHA13eog@mail.gmail.com>
References: <CANxoLDfRgTKK7rJPR14R8zNnmq3Uu1p37KbMYTba8coHA13eog@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi

On Tue, Mar 29, 2016 at 8:09 PM, Akshay Joshi <[email protected]
> wrote:

> Hi All
>
> While implementing Query Tool, I came to know we need error message's with
> some extra information and also I have faced one issue regarding
> asynchronous connection polling. I have added support for formatted error
> message and also fixed the issue.
>
> Attached is the patch file. Please review it and let me know the review
> comments if any.
>

   Apart from above, today I fixed one more issue. In case of user cancel
the running transaction, poll function of psycopg2 returns
psycopg2.extensions.POLL_OK, which is mislead by the caller of the
function, so I have added one flag which will be true if user purposely
cancel the transaction and from our poll function we return new status i.e.
ASYNC_CANCEL_TRANSACTION.

 Attached is the modified patch. Please review it and if it looks good
please commit it.

>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>



-- 
*Akshay Joshi*
*Principal Software Engineer *



*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*


-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Attachments:

  [application/octet-stream] Psycopg_Formatted_error_v2.patch (8.5K, 3-Psycopg_Formatted_error_v2.patch)
  download | inline diff:
diff --git a/web/pgadmin/utils/driver/abstract.py b/web/pgadmin/utils/driver/abstract.py
index 8a23ccb..c2903d1 100644
--- a/web/pgadmin/utils/driver/abstract.py
+++ b/web/pgadmin/utils/driver/abstract.py
@@ -147,6 +147,7 @@ class BaseConnection(object):
     ASYNC_READ_TIMEOUT = 2
     ASYNC_WRITE_TIMEOUT = 3
     ASYNC_NOT_CONNECTED = 4
+    ASYNC_CANCEL_TRANSACTION = 5
 
     @abstractmethod
     def connect(self, **kwargs):
diff --git a/web/pgadmin/utils/driver/psycopg2/__init__.py b/web/pgadmin/utils/driver/psycopg2/__init__.py
index db938bc..a97a853 100644
--- a/web/pgadmin/utils/driver/psycopg2/__init__.py
+++ b/web/pgadmin/utils/driver/psycopg2/__init__.py
@@ -35,7 +35,7 @@ from .keywords import ScanKeyword
 
 _ = gettext
 
-ASYNC_WAIT_TIMEOUT = 0.1  # in seconds or 100 milliseconds
+ASYNC_WAIT_TIMEOUT = 0.01  # in seconds or 10 milliseconds
 
 
 class Connection(BaseConnection):
@@ -102,6 +102,11 @@ class Connection(BaseConnection):
     * messages()
       - Returns the list of messages/notices sends from the PostgreSQL database
         server.
+
+    * _formatted_error_msg(err_obj)
+      - This method is used to parse the psycopg2.Error object and returns the
+        formatted error message.
+
     """
     def __init__(self, manager, conn_id, db, auto_reconnect=True, async=0):
         assert(manager is not None)
@@ -116,6 +121,8 @@ class Connection(BaseConnection):
         self.__async_cursor = None
         self.__async_query_id = None
         self.__backend_pid = None
+        self.formatted_error = None
+        self.is_cancel_transaction = False
 
         super(Connection, self).__init__()
 
@@ -224,6 +231,7 @@ Failed to connect to the database server(#{server_id}) for connection ({conn_id}
 
         self.conn = pg_conn
         self.__backend_pid = pg_conn.get_backend_pid()
+        self.is_cancel_transaction = False
 
         # autocommit flag does not work with asynchronous connections.
         # By default asynchronous connection runs in autocommit mode.
@@ -447,15 +455,17 @@ Attempt to reconnect it failed with the below error:
 
         return True, None
 
-    def execute_async(self, query, params=None):
+    def execute_async(self, query, params=None, formatted_error=False):
         """
         This function executes the given query asynchronously and returns result.
 
         Args:
             query: SQL query to run.
             params: extra parameters to the function
+            formatted_error: if True then function return the formatted error
         """
         status, cur = self.__cursor()
+        self.formatted_error = formatted_error
 
         if not status:
             return False, str(cur)
@@ -472,10 +482,21 @@ Execute (async) for server #{server_id} - {conn_id} (Query-id: {query_id}):\n{qu
         )
 
         try:
+            self.is_cancel_transaction = False
             cur.execute(query, params)
             res = self._wait_timeout(cur.connection, ASYNC_WAIT_TIMEOUT)
         except psycopg2.Error as pe:
-            errmsg = str(pe)
+            if self.formatted_error:
+                # Get the formatted error message
+                errmsg = self._formatted_error_msg(pe)
+            else:
+                if pe.pgerror:
+                    errmsg = pe.pgerror
+                elif pe.diag.message_detail:
+                    errmsg = pe.diag.message_detail
+                else:
+                    errmsg = str(pe)
+
             current_app.logger.error("""
 Failed to execute query (execute_async) for the server #{server_id} - {conn_id}
 (Query-id: {query_id}):\nError Message:{errmsg}
@@ -728,11 +749,15 @@ Failed to reset the connection of the server due to following error:
         if state == psycopg2.extensions.POLL_OK:
             return self.ASYNC_OK
         elif state == psycopg2.extensions.POLL_WRITE:
-            select.select([], [conn.fileno()], [], time)
-            return self.ASYNC_WRITE_TIMEOUT
+            if select.select([], [conn.fileno()], [], time) == ([], [], []):
+                return self.ASYNC_WRITE_TIMEOUT
+            # Call recursively if no timeout
+            self._wait_timeout(conn, time)
         elif state == psycopg2.extensions.POLL_READ:
-            select.select([conn.fileno()], [], [], time)
-            return self.ASYNC_READ_TIMEOUT
+            if select.select([conn.fileno()], [], [], time) == ([], [], []):
+                return self.ASYNC_READ_TIMEOUT
+
+            self._wait_timeout(conn, time)
         else:
             raise psycopg2.OperationalError("poll() returned %s from _wait_timeout function" % state)
 
@@ -751,11 +776,32 @@ Failed to reset the connection of the server due to following error:
         current_app.logger.log(25, """
 Polling result for (Query-id: {query_id})""".format(query_id=self.__async_query_id))
 
-        status = self._wait_timeout(self.conn, ASYNC_WAIT_TIMEOUT)
+        try:
+            status = self._wait_timeout(self.conn, ASYNC_WAIT_TIMEOUT)
+        except psycopg2.Error as pe:
+            if self.formatted_error:
+                # Get the formatted error message
+                errmsg = self._formatted_error_msg(pe)
+            else:
+                if pe.pgerror:
+                    errmsg = pe.pgerror
+                elif pe.diag.message_detail:
+                    errmsg = pe.diag.message_detail
+                else:
+                    errmsg = str(pe)
+            return False, errmsg, None
+
         colinfo = None
         if status == self.ASYNC_OK:
+
+            # if user has cancelled the transaction then changed the status
+            if self.is_cancel_transaction:
+                status = self.ASYNC_CANCEL_TRANSACTION
+                return status, None, colinfo
+
             # Fetch the column information
-            colinfo = [desc for desc in cur.description]
+            if cur.description is not None:
+                colinfo = [desc for desc in cur.description]
 
             if cur.rowcount > 0:
                 result = []
@@ -823,6 +869,9 @@ Polling result for (Query-id: {query_id})""".format(query_id=self.__async_query_
         else:
             if self.connected():
                 status, msg = self.execute_void(query)
+
+                if status:
+                    cancel_conn.is_cancel_transaction = True
             else:
                 status = False
                 msg = gettext("Not connected to the database server.")
@@ -835,6 +884,64 @@ Polling result for (Query-id: {query_id})""".format(query_id=self.__async_query_
         """
         return self.conn.notices if self.conn else []
 
+    def _formatted_error_msg(self, err_obj):
+        """
+        This method is used to parse the psycopg2.Error object
+        and returns the formatted error message.
+
+        Args:
+            err_obj: psycopg2.Error object
+
+        Returns: Formatted error message
+        """
+
+        if err_obj.pgerror:
+            errmsg = err_obj.pgerror
+        elif err_obj.diag.message_detail:
+            errmsg = err_obj.diag.message_detail
+        else:
+            errmsg = str(err_obj)
+
+        errmsg += '********** Error **********\n\n'
+
+        if err_obj.diag.severity is not None \
+                and err_obj.diag.message_primary is not None:
+            errmsg += err_obj.diag.severity + ": " + err_obj.diag.message_primary
+        elif err_obj.diag.message_primary is not None:
+            errmsg += err_obj.diag.message_primary
+
+        if err_obj.diag.sqlstate is not None:
+            if not errmsg[:-1].endswith('\n'):
+                errmsg += '\n'
+            errmsg += gettext('SQL state: ')
+            errmsg += err_obj.diag.sqlstate
+
+        if err_obj.diag.message_detail is not None:
+            if not errmsg[:-1].endswith('\n'):
+                errmsg += '\n'
+            errmsg += gettext('Detail: ')
+            errmsg += err_obj.diag.message_detail
+
+        if err_obj.diag.message_hint is not None:
+            if not errmsg[:-1].endswith('\n'):
+                errmsg += '\n'
+            errmsg += gettext('Hint: ')
+            errmsg += err_obj.diag.message_hint
+
+        if err_obj.diag.statement_position is not None:
+            if not errmsg[:-1].endswith('\n'):
+                errmsg += '\n'
+            errmsg += gettext('Character: ')
+            errmsg += err_obj.diag.statement_position
+
+        if err_obj.diag.context is not None:
+            if not errmsg[:-1].endswith('\n'):
+                errmsg += '\n'
+            errmsg += gettext('Context: ')
+            errmsg += err_obj.diag.context
+
+        return errmsg
+
 
 class ServerManager(object):
     """


view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: [pgAdmin4][Patch] Add formatted error message support in psycopg2
  In-Reply-To: <CANxoLDck1+6BXZvjcQ7bSfGGsnESV74-fWpkuwUsJm8Fuh9GwA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox