Skip to content

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

Merged
micafer merged 7 commits into
apache:trunkfrom
JunaidAhamed-7777:fix/local-noop-lock-errors
Nov 8, 2026
Merged

micafer merged 7 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
Contributor

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.61%. Comparing base (27ce594) to head (fcdc5e3).
⚠️ Report is 8 commits behind head on trunk.

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk    #2204   +/-   ##
=======================================
  Coverage   83.60%   83.61%           
=======================================
  Files         352      352           
  Lines       82138    82161   +23     
  Branches     8814     8816    +2     
=======================================
+ Hits        68671    68694   +23     
  Misses      10594    10594           
  Partials     2873     2873           
Files with missing lines Coverage Δ
libcloud/storage/drivers/local.py 77.95% <100.00%> (ø)
libcloud/test/storage/test_local.py 92.38% <100.00%> (+0.46%) ⬆️
🚀 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
auto-merge was automatically disabled November 8, 2026 06:52

Head branch was pushed to by a user without write access

@micafer
micafer enabled auto-merge November 8, 2026 08:43
@micafer
micafer merged commit e28348a into apache:trunk Nov 8, 2026
17 checks passed
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