Skip to content
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

fix: remove problematic process-shared global variables #2338

Merged
merged 3 commits into from
Mar 10, 2025

Conversation

BoboTiG
Copy link
Owner

@BoboTiG BoboTiG commented Mar 9, 2025

Definitively fixes #1054, and simplify the thinking process while adding more explicit code.

It seems a lot of code, maybe that's not the best approach, but at least it feels more proper & explicit: I added the missed_templates keyword-argument from top calls until the lowest function. It will work either with a simple list (like when used from --get-word, or --check-word), a shared multiprocessing object (like in --render), and it does not break other code since it is None by default.
No more global variables, and I tested with all multiprocessing start methods (fork, forkserver, and spawn): they all work.

Bonus points:

  • --check-words now reports missed templates
  • no more side-effects in tests: each run has its own missed_template value (as seen in test_2_render.py clean-up)

BoboTiG added 2 commits March 9, 2025 12:28
Definitively fix #1054, and simplify the thinking process while adding more explicit code.
[skip ci]
@BoboTiG
Copy link
Owner Author

BoboTiG commented Mar 9, 2025

Side note: While adapting the code to all locales, I saw that English (EN) is not calling the default last template handler function, and so it has/will never report missed templates.

@BoboTiG
Copy link
Owner Author

BoboTiG commented Mar 9, 2025

/cc @lasconic

@BoboTiG BoboTiG merged commit 791900a into master Mar 10, 2025
3 checks passed
@BoboTiG BoboTiG deleted the fix-multiprocessing-shares branch March 10, 2025 21:14
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.

Cannot run tests on macOS
1 participant