Skip to content

Add dkimpy to repo requirements#206

Merged
lavr merged 2 commits intomasterfrom
fix/issue-196-followup
Mar 31, 2026
Merged

Add dkimpy to repo requirements#206
lavr merged 2 commits intomasterfrom
fix/issue-196-followup

Conversation

@lavr
Copy link
Copy Markdown
Owner

@lavr lavr commented Mar 31, 2026

Summary

  • Add dkimpy to requirements/base.txt — was missing after Replace vendored dkim with dkimpy package #205, causing ModuleNotFoundError for dev/CI installs via pip install -r requirements/tests.txt
  • Document that PEM key re-parsing per dkim.sign() call has negligible cost (~0ms vs ~2.5ms RSA operation), confirmed by benchmark of 1000 signs

Context

Review of #205 flagged two issues:

  1. P2 (fix): dkimpy missing from repo requirements — fixed
  2. P2 (investigated): per-call key parsing regression — benchmarked at 0ms overhead, not a real regression

lavr added 2 commits March 31, 2026 23:55
- Add dkimpy to requirements/base.txt (was missing after #205)
- Remove thread-unsafe monkey-patch of dkim.parse_pem_private_key;
  PEM parse cost is negligible vs RSA (~0ms vs ~2.5ms per sign)
- Add test_dkim_sign_after_error for signer recovery after errors
@lavr
Copy link
Copy Markdown
Owner Author

lavr commented Mar 31, 2026

I measured this locally with temporary microbenchmarks. Caching the parsed private key does provide a real speedup, but it is small in practice.

On this machine:

  • For a 1024-bit RSA key, parse_pem_private_key() costs about 0.085 ms, while a full dkim.sign() costs about 2.61 ms. Reusing the parsed key saves about 0.10 ms per message, or roughly 3.9%.
  • For a 2048-bit RSA key, parse_pem_private_key() costs about 0.181 ms, while a full dkim.sign() costs about 13.16 ms. Reusing the parsed key saves about 0.19 ms per message, or roughly 1.5%.

So the overhead is measurable, but it is only a small fraction of total signing time. I do not think that gain justifies the current implementation based on globally monkey-patching dkim.parse_pem_private_key, because that introduces a correctness/concurrency risk that is disproportionate to the performance benefit.

@lavr lavr merged commit 7e8212f into master Mar 31, 2026
10 checks passed
@lavr lavr deleted the fix/issue-196-followup branch April 2, 2026 17:38
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