IGNITE-12867 DBAPI support - #39
sdarlington wants to merge 5 commits into
Conversation
Sorry, something went wrong.
| @@ -0,0 +1,281 @@ | |||
| # | |||
There was a problem hiding this comment.
I am strongly against to put dependency on sqlalchemy to thin client.
Sorry, something went wrong.
There was a problem hiding this 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.
Sorry, something went wrong.
There was a problem hiding this comment.
I suppose that it should be additional module. Just start separate module in separate repo.
Sorry, something went wrong.
There was a problem hiding this 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.
Sorry, something went wrong.
There was a problem hiding this 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
Sorry, something went wrong.
| """ | ||
| self._check_query_started() | ||
|
|
||
| if size is None: |
There was a problem hiding this 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.
Sorry, something went wrong.
| @@ -0,0 +1,87 @@ | |||
| # | |||
| # Copyright 2021 GridGain Systems, Inc. and Contributors. | |||
There was a problem hiding this comment.
Could you please change file header to appropriate one
Sorry, something went wrong.
| threadsafety = 2 | ||
| paramstyle = 'qmark' | ||
|
|
||
| def connect(dsn=None, |
There was a problem hiding this comment.
What if ignite node is out? We should support multiple nodes
Sorry, something went wrong.
Still a way to go, but basically works.