Complete Fetch Phase (`EXTERNAL_LINKS` disposition and `ARROW` format) #598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

varun-edachali-dbx wants to merge 324 commits into sea-migration from ext-links-sea

+1,376 −337

Collaborator

varun-edachali-dbx commented Jun 16, 2025 •

edited

Loading

What type of PR is this?

Feature

Description

Complete the Fetch phase of the SEA implementation for EXTERNAL_LINKS disposition and ARROW_STREAM format. Introduce a new CloudFetchQueue.

Note that this is not yet feature complete, with some known failures in the following domains (for the SEA backend only):

retries
native, complex params
metadata queries

These will be resolved in future PRs.

How is this tested?

Unit tests
E2E Tests
Manually - using the test scripts invoked in examples/experimental/sea_connector_test.py.
N/A

The coverage of the key classes by the unit tests are as below:

Module	Statements	Missing	Coverage	Notes
`backend/sea/queue.py` (SeaCloudFetchQueue and SeaResultSetQueueFactory classes)	92	2	98%	Missing lines are pyarrow import exception handling (lines 10-11) which are not meant to be covered in runtime tests
`backend/sea/result_set.py` (SeaResultSet class)	85	3	96%	Missing lines are pyarrow import exception handling (lines 13-14) and TYPE_CHECKING import (line 17) which are not meant to be covered in runtime tests

Related Tickets & Documents

Design Doc
PECOBLR-553

varun-edachali-dbx added 30 commits

June 11, 2025 06:10


          Merge branch 'exec-resp-norm' into fetch-json-inline

170f339


          Merge branch 'sea-res-set' into fetch-json-inline

40f79b5


          working JSON + INLINE

c038d5a

Signed-off-by: varun-edachali-dbx <[email protected]>


          change to valid table name

3e22c6c

Signed-off-by: varun-edachali-dbx <[email protected]>


          rmeove redundant queue init

716304b

Signed-off-by: varun-edachali-dbx <[email protected]>


          large query results

e96e5b8

Signed-off-by: varun-edachali-dbx <[email protected]>


          Merge branch 'sea-migration' into sea-test-scripts

787f1f7


          remove un-necessary changes

165c4f3

covered by #588

Signed-off-by: varun-edachali-dbx <[email protected]>


          simplify test module

a6e40d0

Signed-off-by: varun-edachali-dbx <[email protected]>


          logging -> debug level

52e3088

Signed-off-by: varun-edachali-dbx <[email protected]>


          change table name in log

641c09b

Signed-off-by: varun-edachali-dbx <[email protected]>


          Merge branch 'sea-migration' into exec-models-sea

8bd12d8


          remove un-necessary changes

ffded6e

Signed-off-by: varun-edachali-dbx <[email protected]>


          remove un-necessary backend cahnges

227f6b3

Signed-off-by: varun-edachali-dbx <[email protected]>


          remove un-needed GetChunksResponse

68657a3

Signed-off-by: varun-edachali-dbx <[email protected]>


          remove un-needed GetChunksResponse

3940eec

only relevant in Fetch phase

Signed-off-by: varun-edachali-dbx <[email protected]>


          reduce code duplication in response parsing

37813ba

Signed-off-by: varun-edachali-dbx <[email protected]>


          reduce code duplication

267c9f4

Signed-off-by: varun-edachali-dbx <[email protected]>


          more clear docstrings

Signed-off-by: varun-edachali-dbx <[email protected]>


          introduce strongly typed ChunkInfo

47fd60d

Signed-off-by: varun-edachali-dbx <[email protected]>


          remove is_volume_operation from response

982fdf2

Signed-off-by: varun-edachali-dbx <[email protected]>


          add is_volume_op and more ResultData fields

9e14d48

Signed-off-by: varun-edachali-dbx <[email protected]>


          Merge branch 'exec-models-sea' into exec-phase-sea

be1997e


          Merge branch 'sea-test-scripts' into exec-phase-sea

e8e8ee7


          add test scripts

05ee4e7

Signed-off-by: varun-edachali-dbx <[email protected]>


          Merge branch 'exec-models-sea' into metadata-sea

3ffa898


          Revert "Merge branch 'sea-migration' into exec-models-sea"

2952d8d

This reverts commit 8bd12d8, reversing
changes made to 030edf8.


          Merge branch 'exec-phase-sea' into metadata-sea

89e2aa0


          Revert "Merge branch 'exec-models-sea' into exec-phase-sea"

cbace3f

This reverts commit be1997e, reversing
changes made to 37813ba.


          change logging level

c075b07

Signed-off-by: varun-edachali-dbx <[email protected]>

jayantsing-db reviewed

View reviewed changes

src/databricks/sql/backend/databricks_client.py Outdated Show resolved Hide resolved

src/databricks/sql/backend/databricks_client.py Outdated Show resolved Hide resolved

src/databricks/sql/backend/sea/backend.py Show resolved Hide resolved

src/databricks/sql/backend/sea/backend.py Outdated Show resolved Hide resolved

src/databricks/sql/backend/sea/models/responses.py Show resolved Hide resolved

src/databricks/sql/backend/sea/queue.py

Comment on lines +180 to +186

    
                          return None

                      next_chunk_index = self._current_chunk_link.next_chunk_index

                      if next_chunk_index is None:

                          self._current_chunk_link = None

                          return None

Contributor

jayantsing-db Jul 14, 2025

in case of partial/corrupt data, where are we throwing user-facing errors?

src/databricks/sql/backend/sea/queue.py Outdated Show resolved Hide resolved

src/databricks/sql/backend/sea/queue.py Outdated Show resolved Hide resolved

src/databricks/sql/utils.py Show resolved Hide resolved

src/databricks/sql/utils.py Show resolved Hide resolved

jayantsing-db requested changes

View reviewed changes

Contributor

jayantsing-db left a comment

Few comments inline.

src/databricks/sql/utils.py Outdated Show resolved Hide resolved

src/databricks/sql/utils.py Show resolved Hide resolved

src/databricks/sql/session.py Show resolved Hide resolved

src/databricks/sql/backend/sea/queue.py Show resolved Hide resolved

src/databricks/sql/backend/sea/queue.py Outdated Show resolved Hide resolved

tests/e2e/common/large_queries_mixin.py Show resolved Hide resolved

varun-edachali-dbx added 4 commits

July 14, 2025 15:49


          remove common constructor for databricks client abc

f7fd1d9

Signed-off-by: varun-edachali-dbx <[email protected]>


          make SEA Http Client instance a private member

510b0a3

Signed-off-by: varun-edachali-dbx <[email protected]>


          make GetChunksResponse model more robust

dd2864b

Signed-off-by: varun-edachali-dbx <[email protected]>


          add link to doc of GetChunk response model

c32b281

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx temporarily deployed to azure-prod

July 14, 2025 10:39

— with

GitHub Actions Inactive

github-actions bot commented Jul 14, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).


          pass result_data instead of "initial links" into SeaCloudFetchQueue

0b1eba5

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx temporarily deployed to azure-prod

July 14, 2025 11:18

— with

GitHub Actions Inactive

github-actions bot commented Jul 14, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).


          move download_manager init into parent CloudFetchQueue

777f7c1

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx temporarily deployed to azure-prod

July 14, 2025 11:30

— with

GitHub Actions Inactive

github-actions bot commented Jul 14, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).


          raise ServerOperationError for no 0th chunk

130b0d3

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod

July 14, 2025 11:42

— with

GitHub Actions Failure

github-actions bot commented Jul 14, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).


          unused iports

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod

July 14, 2025 12:09

— with

GitHub Actions Failure

github-actions bot commented Jul 14, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

varun-edachali-dbx had a problem deploying to azure-prod

July 14, 2025 13:06

— with

GitHub Actions Failure


          Merge branch 'sea-migration' into ext-links-sea

d882c6e

varun-edachali-dbx had a problem deploying to azure-prod

July 15, 2025 01:49

— with

GitHub Actions Failure

github-actions bot commented Jul 15, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).


          return None in case of empty respose

5a43686

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod

July 15, 2025 03:32

— with

GitHub Actions Failure

github-actions bot commented Jul 15, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).


          ensure table is empty on no initial link s

28c6bb1

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx temporarily deployed to azure-prod

July 15, 2025 03:36

— with

GitHub Actions Inactive

github-actions bot commented Jul 15, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet