Skip to content
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.

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

[Feature] [SC-179831] Preliminary support for ranged downloads in SDK #1

Conversation

nickles-lee
Copy link
Owner

@nickles-lee nickles-lee commented Oct 30, 2024

Changes

  • Adds a class that exposes a ranged download as a seekable BinaryIO stream
  • The seekable stream will re-open any closed streams, accounting for some faults and expected socket timeouts due to inactivity, etc.

Known issues

  • [DECO-23875] results in the download being performed in full before the byte array is streamed. This will cause negative performance impacts with this PR until it is fixed.
  • Integration testing is possible via an adhoc script, will follow in a separate PR
  • CI jobs are only run on the DB main fork;

Tests

  • [✅] make test run locally
  • [✅] make fmt applied
  • [🔁] relevant integration tests applied. This'll be added in a separate PR merging onto this branch for reasons

See databricks#807 for CI results

Branch Model

  • All PRs for this feature will be merged onto a common feature branch that will be merged to the main branch in one go.

@nickles-lee nickles-lee changed the title [SC-179831] Preliminary support for ranged downloads in SDK SC-179831 Preliminary support for ranged downloads in SDK Oct 30, 2024
@nickles-lee nickles-lee changed the title SC-179831 Preliminary support for ranged downloads in SDK [Feature] [SC-179831] Preliminary support for ranged downloads in SDK Oct 30, 2024
@@ -5,7 +5,7 @@
from databricks.sdk import azure
from databricks.sdk.credentials_provider import CredentialsStrategy
from databricks.sdk.mixins.compute import ClustersExt
from databricks.sdk.mixins.files import DbfsExt
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is autogen

@@ -813,29 +813,31 @@ def wait_get_app_active(self,
attempt += 1
raise TimeoutError(f'timed out after {timeout}: {status_message}')

def wait_get_app_stopped(self,
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is autogen

@@ -7865,30 +7865,29 @@ def wait_command_status_command_execution_cancelled(
attempt += 1
raise TimeoutError(f'timed out after {timeout}: {status_message}')

def wait_command_status_command_execution_finished_or_error(
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is autogen

@@ -2122,13 +2122,13 @@ class PipelinesAPI:
def __init__(self, api_client):
self._api = api_client

def wait_get_pipeline_idle(
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is autogen

@nickles-lee nickles-lee closed this Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant