feat: provide timeout for each operation#7176
feat: provide timeout for each operation#7176dheeraj12347 wants to merge 0 commit intoapache:mainfrom
Conversation
|
Related Documentation No published documentation to review for changes on this repository. |
core/core/src/raw/ops.rs
Outdated
| /// Default to `false` | ||
| deleted: bool, | ||
| /// The timeout for this operation. | ||
| timeout: Option<Duration>, |
There was a problem hiding this comment.
Thank you, but Layer's behavior should never be injected into ops level.
There was a problem hiding this comment.
Thank you, but
Layer's behavior should never be injected into ops level.
Thank you for the feedback, @Xuanwo. I understand now that the ops level should remain agnostic of Layer behaviors.
To support per-operation timeouts without injecting them into raw/ops.rs, would you recommend using Operator::call with a custom context, or is there a preferred pattern in OpenDAL for passing per-request configurations to Layers? I'll revert the changes to ops.rs and refactor accordingly.
There was a problem hiding this comment.
Thank you, but
Layer's behavior should never be injected into ops level.
I have pushed the refactored TimeoutLayer. It now handles timeouts entirely within the layer, and I've reverted all changes to the core ops.rs. The code is now architecturally clean and passes local checks. Looking forward to your guidance on the best way to re-introduce per-operation overrides!
|
Hi, @dheeraj12347, please review your own PR before requesting a maintainer's review. This PR has now been marked as a low-quality contribution. If no further improvement is provided, I will close it directly. Reasons: This PR does not address the problem, but instead removes many existing comments. If you are unsure how to proceed, please return to the discussion before starting a PR. |
Hi @Xuanwo, I sincerely apologize for the oversight. I see now that my last refactor accidentally removed existing comments and didn't solve the core issue as intended. I am stepping back to re-evaluate the architecture and will restore the deleted documentation immediately. I'll move the technical discussion back to the issue #7168 before making further pushes to ensure I'm aligned with OpenDAL's design principles. Thank you for your patience. |
This PR implements per-operation timeout support as requested in #7168.
Changes:
Added timeout field and with_timeout method to OpRead, OpWrite, OpStat, and OpList in core.
Updated TimeoutLayer to prioritize per-operation timeouts over global defaults.
Fixes: #7168