More FromArgs message by youknowone · Pull Request #6806 · RustPython/RustPython
📝 Walkthrough
Walkthrough
The PR refactors argument handling in the faulthandler and ssl modules to use more type-safe patterns: ArgIntoFloat for numeric arguments with custom error messages, and Either<PyStrRef, ArgBytesLike> for path/bytes union inputs, replacing generic PyObjectRef with specialized type variants and standardized error messaging.
Changes
| Cohort / File(s) | Summary |
|---|---|
Timeout argument handling crates/stdlib/src/faulthandler.rs |
DumpTracebackLaterArgs.timeout field refactored from PyObjectRef to ArgIntoFloat with custom error message. dump_traceback_later now uses into_float() conversion instead of manual downcast/try_index logic. Timeout validation (> 0) and timeout_us computation preserved. |
Path and bytes argument handling crates/stdlib/src/ssl.rs |
LoadVerifyLocationsArgs fields (cafile, capath, cadata) changed from OptionalArg<Option> to OptionalArg<Option<Either<PyStrRef, ArgBytesLike>>>. LoadCertChainArgs updated similarly for certfile and keyfile. Helper functions parse_path_arg and parse_cadata_arg signature updated to accept Either variant. Argument parsing logic adjusted to match on new Either-based inputs and removed vm.is_none checks. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~23 minutes
Possibly related PRs
- Faulthandler #6400: Modifies the same faulthandler timeout argument handling in DumpTracebackLaterArgs and dump_traceback_later.
- FromArgs with error_msg #6804: Introduces FromArgs "error_msg" wiring that enables the pyarg error_msg annotations used throughout these changes.
Suggested reviewers
- ShaharNaveh
- coolreader18
Poem
🐰 Hops with joy
Arguments grow more precise,
Either paths and bytes play nice,
Float conversions, errors clear—
Type safety's victory is here! 🎉
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | ❓ Inconclusive | The title 'More FromArgs message' is vague and generic, using non-descriptive terms that do not convey meaningful information about the changeset's primary objectives. | Revise the title to be more specific and descriptive of the actual changes, such as 'Improve error messages in faulthandler and ssl argument parsing' or 'Add custom error messages to FromArgs structures'. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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.