Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: cloudstack-dev@incubator.apache.org
Received-SPF: neutral (nike.apache.org: local policy)
MIME-Version: 1.0
In-Reply-To: <20121020114954.2E40919468@tyr.zones.apache.org>
References: <20121020114954.2E40919468@tyr.zones.apache.org>
From: David Nalley <david@gnsa.us>
Date: Sat, 20 Oct 2012 18:28:57 -0400
Message-ID: 
 <CAKprHVZ6JvYhuC1X9bRm0ymChhOURKKRbo98EcXewdHvM+hgVw@mail.gmail.com>
Subject: Re: [2/2] git commit: Summary: Replace pymysql in marvin with a
 stable mysql-connector-python
To: cloudstack-dev@incubator.apache.org
Content-Type: text/plain; charset=ISO-8859-1

Prasanna:

You are changing dependencies here - this needs to be brought up and
discussed (and potentially documented.) What are the licensing
implications of this move?

--David

On Sat, Oct 20, 2012 at 7:49 AM,  <tsp@apache.org> wrote:
> Summary: Replace pymysql in marvin with a stable mysql-connector-python
>
> Detail: mysql-connector-python developed by Oracle will replace the MIT
> licensed pymysql. mysql-connector-python is developed by Oracle and is
> more favourable, faster and actively developed.
>
> With this commit - the dbConnection object is also auto managed by
> contextlib. Each transaction requests its own connection rather than
> sharing one single connection for all the test runs.
>
> BUG-ID : None
> Reviewed-by: timeit comparison of pymysql and mysql-connector-python
> Reported-by: dbExceptions and timeouts from Marvin test runs
> Signed-off-by: Prasanna Santhanam <tsp@apache.org> 1350732083 +0530
>
>
> Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo
> Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/355b1529
> Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/355b1529
> Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/355b1529
>
> Branch: refs/heads/master
> Commit: 355b15299323482af06f0e8f0cb32b4d954d9ea2
> Parents: c63f4ea
> Author: Prasanna Santhanam <tsp@apache.org>
> Authored: Sat Oct 20 16:50:51 2012 +0530
> Committer: Prasanna Santhanam <tsp@apache.org>
> Committed: Sat Oct 20 16:57:06 2012 +0530
>
> ----------------------------------------------------------------------
>  tools/marvin/marvin/dbConnection.py |   63 +++++++++++-------------------
>  tools/marvin/setup.py               |    2 +-
>  2 files changed, 24 insertions(+), 41 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/355b1529/tools/marvin/marvin/dbConnection.py
> ----------------------------------------------------------------------
> diff --git a/tools/marvin/marvin/dbConnection.py b/tools/marvin/marvin/dbConnection.py
> index 1992f80..eb01d73 100644
> --- a/tools/marvin/marvin/dbConnection.py
> +++ b/tools/marvin/marvin/dbConnection.py
> @@ -15,59 +15,42 @@
>  # specific language governing permissions and limitations
>  # under the License.
>
> -import pymysql
> +import mysql
> +import contextlib
> +from mysql import connector
> +from mysql.connector import errors
> +from contextlib import closing
>  import cloudstackException
>  import sys
>  import os
> -import traceback
> +
>  class dbConnection(object):
>      def __init__(self, host="localhost", port=3306, user='cloud', passwd='cloud', db='cloud'):
>          self.host = host
>          self.port = port
> -        self.user = user
> +        self.user = str(user) #Workaround: http://bugs.mysql.com/?id=67306
>          self.passwd = passwd
>          self.database = db
>
> -        try:
> -            self.db = pymysql.Connect(host=host, port=port, user=user, passwd=passwd, db=db)
> -        except:
> -            traceback.print_exc()
> -            raise cloudstackException.InvalidParameterException(sys.exc_info())
> -
> -    def __copy__(self):
> -        return dbConnection(self.host, self.port, self.user, self.passwd, self.database)
> -
> -    def close(self):
> -        try:
> -            self.db.close()
> -        except:
> -            pass
> -
> -    def execute(self, sql=None):
> +    def execute(self, sql=None, params=None):
>          if sql is None:
>              return None
> -
> +
>          resultRow = []
> -        cursor = None
> -        try:
> -            # commit to restart the transaction, else we don't get fresh data
> -            self.db.commit()
> -            cursor = self.db.cursor()
> -            cursor.execute(sql)
> -
> -            result = cursor.fetchall()
> -            if result is not None:
> -                for r in result:
> -                    resultRow.append(r)
> -            return resultRow
> -        except pymysql.MySQLError, e:
> -            raise cloudstackException.dbException("db Exception:%s"%e)
> -        except:
> -            raise cloudstackException.internalError(sys.exc_info())
> -        finally:
> -            if cursor is not None:
> -                cursor.close()
> -
> +        with contextlib.closing(mysql.connector.connect(host=self.host, port=self.port, user=self.user, password=self.passwd, db=self.database)) as conn:
> +            conn.autocommit = True
> +            with contextlib.closing(conn.cursor(buffered=True)) as cursor:
> +                cursor.execute(sql, params)
> +                try:
> +                    result = cursor.fetchall()
> +                except errors.InterfaceError:
> +                    #Raised on empty result - DML
> +                    result = []
> +                    if result:
> +                        [resultRow.append(r) for r in result]
> +
> +        return resultRow
> +
>      def executeSqlFromFile(self, fileName=None):
>          if fileName is None:
>              raise cloudstackException.InvalidParameterException("file can't not none")
>
> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/355b1529/tools/marvin/setup.py
> ----------------------------------------------------------------------
> diff --git a/tools/marvin/setup.py b/tools/marvin/setup.py
> index c9841f3..118d6ba 100644
> --- a/tools/marvin/setup.py
> +++ b/tools/marvin/setup.py
> @@ -37,7 +37,7 @@ setup(name="Marvin",
>        packages=["marvin", "marvin.cloudstackAPI", "marvin.sandbox", "marvin.sandbox.advanced", "marvin.sandbox.basic"],
>        license="LICENSE.txt",
>        install_requires=[
> -          "pymysql",
> +          "mysql-connector-python",
>            "paramiko",
>            "nose",
>            "unittest-xml-reporting"
>