Skip to content

Propagate local storage upload errors when locking is disabled - #2204

Open
JunaidAhamed-7777 wants to merge 5 commits into
apache:trunkfrom
JunaidAhamed-7777:fix/local-noop-lock-errors
Open

JunaidAhamed-7777 wants to merge 5 commits into
apache:trunkfrom
JunaidAhamed-7777:fix/local-noop-lock-errors

Conversation

@JunaidAhamed-7777

@JunaidAhamed-7777 JunaidAhamed-7777 commented Nov 1, 2026 •

Copy link
Copy Markdown

Description

NoOpLockLocalStorage.__exit__ returns the exception object. A truthy return suppresses that exception, so disabling locking can cause a failed upload to report success. Return False so the original error propagates.

The regressions cover a source stream that yields data and then raises OSError, a missing source file uploaded over an existing destination, and a successful streamed upload with locking disabled.

Reproduction

On trunk at bd9cbb3e393bc42f57684481e7c62e9293007584, Python 3.12.14 on Linux:

import tempfile
from libcloud.storage.drivers.local import LocalStorageDriver

def chunks():
    yield b"partial data"
    raise OSError("source stream failed")

with tempfile.TemporaryDirectory() as root:
    driver = LocalStorageDriver(root, ex_use_locking=False)
    container = driver.create_container("example")
    obj = driver.upload_object_via_stream(chunks(), container, "object")
    print(obj.size)  # 12; the OSError was suppressed

With this change, the original OSError propagates. This does not change partial-file cleanup or the locking-enabled implementation. The bug report is included here; there is no separate tracking issue.

Status

Draft. Human review and hosted CI are pending.

Validation

  • Both error regressions fail on the unmodified production code; the success control passes.
  • pytest -q libcloud/test/storage/test_local.py libcloud/test/storage/test_base.py: 35 passed.
  • Black, isort, flake8 and git diff --check pass for the changed files.
  • Storage-package Pylint passes using the repository's error-only configuration and driver plugin.
  • Full provider tests, other Python versions and live cloud integration tests were not run.

Checklist

  • Code linting (scope above)
  • Documentation (no API addition)
  • Tests
  • ICLA (not signed; small bug fix)

Apache-ai: Code and regression tests prepared with OpenAI Codex.
@JunaidAhamed-7777
JunaidAhamed-7777 marked this pull request as ready for review November 1, 2026 04:49
@micafer

micafer commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Hi! The GitHub Actions workflow runs for this PR have expired because they were awaiting maintainer approval for more than 30 days.

Could you please push a new commit to your PR branch (an empty commit is fine) to trigger fresh workflow runs?

I'll approve the new runs once they appear. Thanks!

@codecov-commenter

codecov-commenter commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.59%. Comparing base (647d5a7) to head (782c7c1).

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk    #2204   +/-   ##
=======================================
  Coverage   83.58%   83.59%           
=======================================
  Files         352      352           
  Lines       82131    82154   +23     
  Branches     8812     8814    +2     
=======================================
+ Hits        68649    68672   +23     
  Misses      10607    10607           
  Partials     2875     2875           
Files with missing lines Coverage Δ
libcloud/storage/drivers/local.py 76.40% <100.00%> (ø)
libcloud/test/storage/test_local.py 92.09% <100.00%> (+0.49%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@micafer

micafer commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Please add the entry in the changelog.

@micafer micafer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@micafer
micafer enabled auto-merge November 7, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants