You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/05/26 14:30:43 UTC

[GitHub] [ignite-python-thin-client] sdarlington opened a new pull request #39: IGNITE-12867 DBAPI support

sdarlington opened a new pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39


   Still a way to go, but basically works.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] ivandasch commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r663204115



##########
File path: pyignite/dbapi/__init__.py
##########
@@ -0,0 +1,87 @@
+#
+# Copyright 2021 GridGain Systems, Inc. and Contributors.
+#
+# Licensed under the GridGain Community Edition License (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from .dbclient import DBClient
+from .errors import *
+from .. import constants
+from urllib.parse import urlparse, parse_qs
+
+apiLevel = '2.0'
+threadsafety = 2
+paramstyle = 'qmark'
+
+def connect(dsn=None,

Review comment:
       What if ignite node is out? We should support multiple nodes
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] ivandasch commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r665091716



##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       I suppose that it should be additional module. Just start separate module in separate repo. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] ivandasch commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r665093071



##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       We can of course rearrange repo, but it is not a hood idea, imho. Currently there is not any options to make it "optional". Just create new module pyignite-sqlalchemy and develope it independently.

##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       We can of course rearrange repo, but it is not a good idea, imho. Currently there is not any options to make it "optional". Just create new module pyignite-sqlalchemy and develope it independently.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] ivandasch commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r663203280



##########
File path: pyignite/dbapi/__init__.py
##########
@@ -0,0 +1,87 @@
+#
+# Copyright 2021 GridGain Systems, Inc. and Contributors.

Review comment:
       Could you please change file header to appropriate one




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] ivandasch commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r663198503



##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       I am strongly against to put dependency on sqlalchemy to thin client.  




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] sdarlington commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
sdarlington commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r663762503



##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       I don't think it should be a _mandatory_ requirement, but support for SQLAlchemy is important. I may yet break this out into a separate PR.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] ivandasch commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r665094152



##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       And sqlalchemy is not important at all, it is just one of many others ORM, and you can barely gind any db driver, that contains anything related to ORM in main repo

##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       And sqlalchemy is not important at all, it is just one of many others ORM, and you can barely find any db driver, that contains anything related to ORM in main repo




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] sdarlington commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
sdarlington commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r663762503



##########
File path: pyignite/dbapi/sqlalchemy.py
##########
@@ -0,0 +1,281 @@
+#

Review comment:
       I don't think it should be a _mandatory_ requirement, but support for SQLAlchemy is important. I may yet break this out into a separate PR.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite-python-thin-client] ivandasch commented on a change in pull request #39: IGNITE-12867 DBAPI support

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #39:
URL: https://github.com/apache/ignite-python-thin-client/pull/39#discussion_r663202891



##########
File path: pyignite/dbapi/dbcursor.py
##########
@@ -0,0 +1,140 @@
+#
+# Copyright 2021 GridGain Systems, Inc. and Contributors.
+#
+# Licensed under the GridGain Community Edition License (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from ..cursors import SqlCursor
+
+class DBCursor(object):
+
+    def __init__(self, connection):
+        self.connection = connection
+        self.cursor = None
+        self.rowcount = -1
+    
+    @property
+    def description(self):
+#         columns =  self._columns
+#         types =  [ bool ]
+
+        return [
+            [name, None, None, None, None, None, True]
+            for name in self._columns
+#             for name, type_code in zip(columns, types)
+        ]
+
+    def close(self):
+        """
+        Close the cursor now. The cursor will be unusable from this point
+        forward; an :data:`~pyignite.dbapi.Error` (or subclass)
+        exception will be raised if any operation is attempted with the
+        cursor.
+        """
+#         self.connection.disconnect()
+#         self._state = self._states.CURSOR_CLOSED
+
+#         try:
+#             # cursor can be already closed
+#             self.connection.cursors.remove(self)
+#         except ValueError:
+#             pass
+
+    def execute(self, operation, parameters=None):
+        """
+        Prepare and execute a database operation (query or command).
+
+        :param operation: query or command to execute.
+        :param parameters: sequence or mapping that will be bound to
+                           variables in the operation.
+        :return: None
+        """
+        self.cursor = self.connection.sql(operation, query_args=parameters, include_field_names=True)
+        self._columns = next(self.cursor)
+
+    def executemany(self, operation, seq_of_parameters):
+        """
+        Prepare a database operation (query or command) and then execute it
+        against all parameter sequences found in the sequence
+        `seq_of_parameters`.
+
+        :param operation: query or command to execute.
+        :param seq_of_parameters: sequences or mappings for execution.
+        :return: None
+        """
+        pass
+
+    def fetchone(self):
+        """
+        Fetch the next row of a query result set, returning a single sequence,
+        or None when no more data is available.
+
+        :return: the next row of a query result set or None.
+        """
+        if self.cursor is not None:
+            return next(self.cursor)
+        else:
+            return None
+
+    def fetchmany(self, size=None):
+        """
+        Fetch the next set of rows of a query result, returning a sequence of
+        sequences (e.g. a list of tuples). An empty sequence is returned when
+        no more rows are available.
+
+        :param size: amount of rows to return.
+        :return: list of fetched rows or empty list.
+        """
+        self._check_query_started()
+
+        if size is None:

Review comment:
       I suppose you should use low level API 
   `pyignite.api.sql.sql_fields`
   `pyignite.api.sql.sql_fields_cursor_get_page`
   
   You can open cursor using the first call and retrieve next pages using the next call. Please, see how current cursors are implemented.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org