Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 19, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved error clarity for fault handler timeout arguments with explicit guidance that timeout values must be numeric (int or float).
  • Improvements

    • Enhanced SSL module to accept both string and bytes-like inputs for certificate file paths, key file paths, and certificate verification locations, providing greater input flexibility for certificate chain and verification operations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

📝 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone marked this pull request as ready for review January 20, 2026 01:24
@youknowone youknowone merged commit ef2c0a6 into RustPython:main Jan 20, 2026
13 checks passed
@youknowone youknowone deleted the fromargs-msg branch January 20, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant