posix.link, termios.error, warn() error, closefd=False for std in/out by youknowone · Pull Request #6942 · RustPython/RustPython
📝 Walkthrough
Walkthrough
This PR updates error handling and OS-specific function behavior across multiple stdlib modules. Changes include refactoring termios error construction to use a structured pattern, adding follow_symlinks parameter support to os.link with platform-specific implementations, propagating warning errors instead of silencing them, and adjusting file descriptor closure behavior for standard IO streams.
Changes
| Cohort / File(s) | Summary |
|---|---|
Error Construction Refactor crates/stdlib/src/termios.rs |
Replaces generic exception creation with structured os_subtype_error pattern, updating parameter arrangement to supply OS-specific errno separately from error message. |
OS Link Enhancement crates/vm/src/stdlib/os.rs |
Adds follow_symlinks parameter to link function; implements platform-specific behavior using libc::linkat on Unix with flag control, and conditional NotImplementedError on non-Unix platforms when follow_symlinks is false. Updates support metadata to advertise conditional availability. |
Error Propagation crates/vm/src/stdlib/warnings.rs |
Changes warning error handling from ignored (let _ =) to propagated (?), allowing failures to contribute to PyResult flow. |
File Descriptor Management crates/vm/src/vm/mod.rs |
Adds closefd: false to OpenArgs for standard IO wrappers to prevent closing underlying file descriptors on drop. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
- Integrate OSError creations into OSErrorBuilder #6443: Centralizes OSError/OSErrorBuilder exception construction that the termios refactor aligns with.
- new_last_{os,errno}_error #6381: Refactors errno/os error helpers (new_last_errno_error) that relate to the termios error handling pattern changes.
Poem
🐰 Errors now flow where once they were hushed,
Symlinks dance conditional, platform-blessed,
File descriptors cling to life as they should,
A refactored warren of errno so good! ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title directly lists all four main changes made in the PR: posix.link enhancement, termios.error refactoring, warn() error propagation, and closefd=False for standard I/O. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.