feat(csharp/src/Drivers/Databricks): BaseDatabricksReader #2842
Conversation
toddmeng-db
force-pushed
the
toddmeng-db/base-status-poller
branch
from
235df22 to
29bc9ff
Compare
toddmeng-db
changed the title
| [InlineData(true, "CloudFetch enabled")] | ||
| [InlineData(false, "CloudFetch disabled")] | ||
| [InlineData(true, "CloudFetch enabled")] // TODO: test cloudfetch disabled | ||
| public async Task StatusPollerKeepsQueryAlive(bool useCloudFetch, string configName) |
There was a problem hiding this comment.
Removes cloudfetch heartbeat test: cloudfetch URLs are fetched during CloudFetchReader initialization, and the URL expire swithin 15 minutes. Delaying for 30 minutes will result in attempting to read expired urls. This makes testing this really difficult.
@jadewang-db Do we need to do anything about this url expiration?
There was a problem hiding this comment.
Discussed offline: we'll need to seperately take a look into handling url expiration, then we should re-enable this test for cloudfetch.
toddmeng-db
force-pushed
the
toddmeng-db/base-status-poller
branch
from
440c1b2 to
a7d6ef3
Compare
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks! Can you take care of a couple of super-nitpicky formatting issues?
| isDisposed = true; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
nit: extra blank line
| this.schema = schema; | ||
| this.isLz4Compressed = isLz4Compressed; | ||
| this.statement = statement; | ||
| if (statement.DirectResults != null && !statement.DirectResults.ResultSet.HasMoreRows) { |
There was a problem hiding this comment.
nit: please put open brace on new line
3 participants
Handles shared reading logic (Status Polling logic) for DatabricksReader and CloudFetchReader. Also modifies polling lifecycle to begin during Reader initialization instead of first time
ReadNextRecordBatchAsyncis calledThere is still a slight gap when we are not polling for results immediately after query execution completes (here); can be addressed in a later PR