-
-
Notifications
You must be signed in to change notification settings - Fork 3
Fix race condition on GitHub authors #177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
👋 Hello @glenn-jocher, thank you for submitting a -✅ Define a Purpose: Clearly explain the purpose of your fix or feature in your PR description, and link to any relevant issues. Ensure your commit messages are clear, concise, and adhere to the project's conventions. For more guidance, please refer to our Contributing Guide. Don't hesitate to leave a comment if you have any questions. Thank you for contributing to Ultralytics! 🚀 |
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 PR Review
Made with ❤️ by Ultralytics Actions
Good refactor overall: moving author resolution to a single main-process pass and having workers consume pre-resolved authors should eliminate the cache write race.
Main items to address: ensure deterministic cache writes by iterating emails in sorted order, add timeouts to GitHub network calls to avoid hanging builds, and normalize/handle empty email keys to preserve prior default_author behavior.
💬 Posted 4 inline comments
|
Merged and looking great, @glenn-jocher — thank you for pushing this through. This PR is a solid quality win for the mkdocs pipeline: pre-resolving authors in the main process neatly eliminates parallel cache write conflicts, the
This change feels exactly like that: small, thoughtful engineering that quietly lifts everyone’s day-to-day—more reliable parallel docs builds, fewer flaky failures, and smoother postprocessing at scale. Appreciate the care and craftsmanship you bring to the Ultralytics ecosystem. |
PR #177 removed `default_author` from `process_html()` but `main.py` still passed it, causing silent failures for users running `mkdocs build`. The fix aligns `main.py` with `postprocess.py` by calling `resolve_all_authors()` during `on_config()` instead of passing `default_author` to `process_html()`.
🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
🚀 Improves MkDocs postprocessing stability and performance by pre-resolving GitHub authors once (avoiding parallel cache write conflicts) and bumping the plugin version to
0.2.4.📊 Key Changes
__version__updated from0.2.3→0.2.4.postprocess_site()now callsresolve_all_authors()before spawning worker processes, preventing race conditions when writing the authors cache file.process_html()andget_git_info()no longer resolve authors on-demand per file.get_git_info()now expects authors to already be present ingit_data(precomputed).load_author_cache()/save_author_cache()utilities formkdocs_github_authors.yaml.TIMEOUT = 10) and lazy default avatar resolution (get_default_avatar()), reducing risk of hangs and repeated network calls.🎯 Purpose & Impact
resolve_all_authors()), so code paths expecting per-file author resolution during HTML processing will no longer run—authors must already be present ingit_data📌.