Skip to content

Conversation

@yuanjieding-db
Copy link
Collaborator

What changes are proposed in this pull request?

WHAT

  • This PR fixes the issue that the old retry mechanism failed to identify the retriable errors.
    • It has done that by providing a new mechanism to support distinguishing retriable and non-retriable errors solely by its status code, which is the expected behavior when calling _retry_cloud_idempotent_operation.
    • The old _retry_idempotent_operation is removed because it is out of use and can be misleading.
  • The PR also refactors the test_files, which are the unit tests for the FilesExt class.
    • Added typing for all methods and argument lists
    • Introduced UploadTestCase as the base class for upload test cases, in order to reduce the code duplication
    • Renamed all chunk to part, to align the terminology with the HTTP API.

WHY

  • The previous implementation utilizes error mappers that's designed for calling Databricks APIs (aka internal APIs), which the retry functions relies on to determine whether the requests is retriable or not. However, when calling the uploading API, it could be calling a CSP API (aka external API), which we wish to distinguish whether it is retriable solely on it's status code.
  • Refactor of unit test file is done to improve its readability and maintainability.

How is this tested?

Unit tests.

ALWAYS ANSWER THIS QUESTION: Answer with "N/A" if tests are not applicable
to your PR (e.g. if the PR only modifies comments). Do not be afraid of
answering "Not tested" if the PR has not been tested. Being clear about what
has been done and not done provides important context to the reviewers.

@yuanjieding-db yuanjieding-db force-pushed the yuanjieding/experimental-files-ext branch from 91344b6 to 75b5700 Compare July 18, 2025 13:19
@yuanjieding-db yuanjieding-db force-pushed the yuanjieding/experimental-files-ext branch from 75b5700 to 7069f16 Compare July 18, 2025 13:23
@github-actions
Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-py

Inputs:

  • PR number: 1009
  • Commit SHA: 7069f16fbed5fa2009acf528c5fd3b0ad348f04f

Checks will be approved automatically on success.

@yuanjieding-db yuanjieding-db enabled auto-merge July 18, 2025 13:37
@yuanjieding-db yuanjieding-db added this pull request to the merge queue Jul 18, 2025
@parthban-db parthban-db removed this pull request from the merge queue due to a manual request Jul 18, 2025
@yuanjieding-db yuanjieding-db changed the title Yuanjieding/experimental files ext Fix FilesExt failed to retry on server side error issue Jul 18, 2025
@yuanjieding-db yuanjieding-db added this pull request to the merge queue Jul 18, 2025
Merged via the queue into main with commit 436a999 Jul 18, 2025
17 checks passed
@yuanjieding-db yuanjieding-db deleted the yuanjieding/experimental-files-ext branch July 18, 2025 14:31
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.

3 participants