Skip to content

Conversation

@dheeraj12347
Copy link

This PR is a complete refactor of the TimeoutLayer to address the architectural feedback from my previous attempt in #7176.

The Problem: My previous PR attempted to add timeout parameters to the operation arguments (ops), which leaked layer logic into the core.

The Solution: This version uses the Decorator pattern to wrap the futures directly within the TimeoutAccessor and TimeoutWrapper.

Key Changes:

Encapsulation: All tokio::time::timeout logic is now internal to the layer.

Zero Core Changes: No modifications were made to opendal_core::raw::ops.

Standardized Errors: Timeout errors now include duration context and are marked as temporary to support retry layers.

Verified: Included unit tests to confirm that operation timeouts trigger correctly.

I would appreciate a fresh review of this approach.

@dheeraj12347 dheeraj12347 requested a review from Xuanwo as a code owner February 4, 2026 05:42
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels Feb 4, 2026
@dheeraj12347
Copy link
Author

I've noticed several failures in the Behavior Tests (Node.js/Python) and the Ocaml documentation build. Since my changes are strictly encapsulated within the Rust TimeoutLayer and do not modify any core ops or bindings, I suspect these are flaky tests or CI-specific issues. Could a maintainer please help verify if a re-run is needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/feat The PR implements a new feature or has a title that begins with "feat" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant