feat: FIR-43722 add streaming to python sdk #424
Conversation
stepansergeevitch
changed the title
src/firebolt/db/cursor.py
Outdated
| in_memory_row_set_type = InMemoryRowSet | ||
| streaming_row_set_type = StreamingRowSet |
There was a problem hiding this comment.
Any particular reason to make those dynamic?
There was a problem hiding this comment.
To refactor our the logic of initializing the row_set based on if we're streaming or not. Also maybe for readability
There was a problem hiding this comment.
It feels a bit overengineered for the purpose. These two are used in a single method, which in turn is used only in _do_execute. And I don't really think we expect them to change often so I'm not sure of the benefit here.
A one line can let us get rid of two attributes, a method and a base class method, which in turn would improve readablility:
self._row_set = StreamingRowSet() if streaming else InMemoryRowSet()
| self.requires_response_body = True | ||
| response = yield self._make_auth_request() | ||
| self.requires_response_body = False |
There was a problem hiding this comment.
This just feels really awkward. Maybe let's rethink the whole requires_response_body after this PR.
src/firebolt/db/cursor.py
Outdated
| in_memory_row_set_type = InMemoryRowSet | ||
| streaming_row_set_type = StreamingRowSet |
There was a problem hiding this comment.
It feels a bit overengineered for the purpose. These two are used in a single method, which in turn is used only in _do_execute. And I don't really think we expect them to change often so I'm not sure of the benefit here.
A one line can let us get rid of two attributes, a method and a base class method, which in turn would improve readablility:
self._row_set = StreamingRowSet() if streaming else InMemoryRowSet()
|
2 participants
Added streaming support for python sdk
integer,bigint,double precision,numericexecute_streamfor asynchronous and synchronous cursors V2