You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ts...@apache.org on 2012/10/20 13:49:54 UTC

[2/2] git commit: Summary: Replace pymysql in marvin with a stable mysql-connector-python

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 <ts...@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 <ts...@apache.org>
Authored: Sat Oct 20 16:50:51 2012 +0530
Committer: Prasanna Santhanam <ts...@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"


mysql-connector-python for marvin (was Re: [2/2] git commit: Summary: Replace pymysql with mysql-connector-python)

Posted by Prasanna Santhanam <ts...@apache.org>.
On Sat, Oct 20, 2012 at 08:20:30PM -0400, David Nalley wrote:
> On Sat, Oct 20, 2012 at 7:19 PM, Rohit Yadav
> <ro...@citrix.com> wrote:
> > Hi David, I think there should not be any issues. It's a runtime
> > dependency just like pymysql and obtainable from cheeseshop/pypi
> > using pip/easy_install and while building/installing/running
> > marvin (http://pypi.python.org/pypi/mysql-connector-python). It's
> > the same case with other deps like nose, unittest-xml-reporting
> > etc.
> >
> > I think it should be used as it's developed and maintained by the
> > MySQL server people and there is nothing we're introducing.
> > mysql-connector-python and MySQL server are both released under
> > GPLv2, pymysql have their own license:
> > https://github.com/petehunt/PyMySQL/blob/master/LICENSE
> >
> 
> 
> So there are a number of potential problems - GPL/LGPL/copyleft
> dependencies can be problematic and even forbidden. Moreover it may
> inject requirements on us to report it in NOTICE or LICENSE.  See
> Chips note about an earlier problem around this:
> http://markmail.org/message/huevw4ur73a64b5c
> 
> We _HAVE_ to handle this on an ongoing basis, an audit after the
> fact is too expensive in terms of time and effort. If anyone
> adds/subtracts/changes a dependency, we need to discuss it, verify
> that we can use it, and document it.

Hi David,

mysql-connector-python is nothing but a cousin to mysql-connector-java
that we earlier had resolved licensing issues with. Aside from the
reason for changing the connector I didn't see a problem in having
this as a runtime dependency since the FOSS exception added to all
mysql -server/client/connector projects as noted below:

http://www.mysql.com/about/legal/licensing/foss-exception/

allows Apache projects to include these connectors. It should probably
be more acceptable than pymysql that wasn't licensed conveniently like
this.

If there are any follow-ups / tasks / release notes to be taken up to
allow its inclusion, please let me know. I will take those up.

Sorry for not bringing this earlier to the lists. But in my haste to
fix the issue I forget to take guidance on the licensing and legal
problems that you guys tirelessly worked out earlier.

No other harm intended.


-- 
Prasanna.,

Re: [2/2] git commit: Summary: Replace pymysql in marvin with a stable mysql-connector-python

Posted by David Nalley <da...@gnsa.us>.
On Sat, Oct 20, 2012 at 7:19 PM, Rohit Yadav <ro...@citrix.com> wrote:
> Hi David, I think there should not be any issues. It's a runtime dependency just like pymysql and obtainable from cheeseshop/pypi using pip/easy_install and while building/installing/running marvin (http://pypi.python.org/pypi/mysql-connector-python). It's the same case with other deps like nose, unittest-xml-reporting etc.
>
> I think it should be used as it's developed and maintained by the MySQL server people and there is nothing we're introducing. mysql-connector-python and MySQL server are both released under GPLv2, pymysql have their own license: https://github.com/petehunt/PyMySQL/blob/master/LICENSE
>


So there are a number of potential problems - GPL/LGPL/copyleft
dependencies can be problematic and even forbidden. Moreover it may
inject requirements on us to report it in NOTICE or LICENSE.
See Chips note about an earlier problem around this:
http://markmail.org/message/huevw4ur73a64b5c

We _HAVE_ to handle this on an ongoing basis, an audit after the fact
is too expensive in terms of time and effort. If anyone
adds/subtracts/changes a dependency, we need to discuss it, verify
that we can use it, and document it.

RE: [2/2] git commit: Summary: Replace pymysql in marvin with a stable mysql-connector-python

Posted by Rohit Yadav <ro...@citrix.com>.
Hi David, I think there should not be any issues. It's a runtime dependency just like pymysql and obtainable from cheeseshop/pypi using pip/easy_install and while building/installing/running marvin (http://pypi.python.org/pypi/mysql-connector-python). It's the same case with other deps like nose, unittest-xml-reporting etc.

I think it should be used as it's developed and maintained by the MySQL server people and there is nothing we're introducing. mysql-connector-python and MySQL server are both released under GPLv2, pymysql have their own license: https://github.com/petehunt/PyMySQL/blob/master/LICENSE

Regards.

________________________________________
From: David Nalley [david@gnsa.us]
Sent: Sunday, October 21, 2012 3:58 AM
To: cloudstack-dev@incubator.apache.org
Subject: Re: [2/2] git commit: Summary: Replace pymysql in marvin with a stable mysql-connector-python

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,  <ts...@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 <ts...@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 <ts...@apache.org>
> Authored: Sat Oct 20 16:50:51 2012 +0530
> Committer: Prasanna Santhanam <ts...@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"
>

Re: [2/2] git commit: Summary: Replace pymysql in marvin with a stable mysql-connector-python

Posted by David Nalley <da...@gnsa.us>.
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,  <ts...@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 <ts...@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 <ts...@apache.org>
> Authored: Sat Oct 20 16:50:51 2012 +0530
> Committer: Prasanna Santhanam <ts...@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"
>