Skip to content

Fix: REMOVE staging operation no longer requires staging_allowed_local_path (#726)#846

Merged
vikrantpuppala merged 1 commit into
mainfrom
fix/726-remove-no-staging-path
Jul 8, 2026
Merged

Fix: REMOVE staging operation no longer requires staging_allowed_local_path (#726)#846
vikrantpuppala merged 1 commit into
mainfrom
fix/726-remove-no-staging-path

Conversation

@vikrantpuppala

Copy link
Copy Markdown
Contributor

Summary

Fixes #726.

A REMOVE staging operation deletes a remote resource and never touches the local filesystem, yet Cursor._handle_staging_operation raised:

You must provide at least one staging_allowed_local_path when initialising a connection to perform ingestion commands

before dispatching to any handler when staging_allowed_local_path was None. This forced callers to pass a dummy path (e.g. "/") just to run a REMOVE, even though the value is never used for that operation — the reporter observed they could PUT via __input_stream__ without a path, but REMOVE still demanded one.

Root cause

In src/databricks/sql/client.py, _handle_staging_operation validated staging_allowed_local_path for every non-streaming operation before dispatch. The REMOVE branch then explicitly popped local_file because "Local file isn't needed to remove a remote resource" — confirming the requirement was spurious for REMOVE.

Fix

Handle REMOVE early, right after the existing __input_stream__ PUT bypass and before the staging_allowed_local_path validation block. GET and local-file PUT continue to require staging_allowed_local_path.

Test plan

Added tests/unit/test_staging_remove.py:

  • test_remove_without_staging_allowed_local_path — the repro: REMOVE with staging_allowed_local_path=None now dispatches to _handle_staging_remove (no local_file forwarded) instead of raising. Fails before the fix, passes after.
  • test_remove_with_staging_allowed_local_pathREMOVE still works when a path is provided.
  • test_get_still_requires_staging_allowed_local_path — regression guard: GET with None still raises the original error.
  • test_unsupported_operation_still_errors — unknown operations still raise.

Verification:

  • poetry run python -m pytest tests/unit/test_staging_remove.py tests/unit/test_streaming_put.py — 8 passed
  • poetry run python -m pytest tests/unit/test_client.py — 128 passed, 2 skipped
  • Full unit suite (-m "not realkernel") — 869 passed, 4 skipped
  • poetry run black — no changes needed

Added a CHANGELOG entry under a new Unreleased section.

Closes #726

…726)

A REMOVE staging operation deletes a remote resource and never touches
the local filesystem, yet _handle_staging_operation raised a
ProgrammingError demanding staging_allowed_local_path before dispatching
to any handler. This forced callers to pass a dummy path (e.g. "/") just
to run a REMOVE, even though the value is never used.

Handle REMOVE before the staging_allowed_local_path validation block,
mirroring how the __input_stream__ PUT path already bypasses it. GET and
local-file PUT continue to require staging_allowed_local_path.

Closes #726

Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
@vikrantpuppala vikrantpuppala added this pull request to the merge queue Jul 8, 2026
Merged via the queue into main with commit ea9bf86 Jul 8, 2026
53 checks passed
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.

REMOVE operation requires staging_allowed_local_path to be set, even though no local file is required

2 participants