Fix IRC mirror timeout when relaying messages - #917
Conversation
|
@Niloth-p would you be up for taking a look at this PR? Relevant CZO discussion here#zulip cloud > IRC mirror issues. |
Niloth-p
left a comment
There was a problem hiding this comment.
Thanks for the PR! LGTM apart from a couple of possible improvements. Please do fix the linting errors in the second commit.
| send(line) | ||
|
|
||
| z2i = mp.Process(target=self.zulip_client.call_on_each_message, args=(forward_to_irc,)) | ||
| def fresh_session() -> None: |
There was a problem hiding this comment.
The name fresh_session is not very readable when being passed to mp.Process. Maybe something like forward_zulip_to_irc might be better.
* zulip/integrations/bridge_with_irc/irc_mirror_backend.py (fresh_session): New procedure. Ensure forked processes have a clean client to ensure they don't share the SSL sockets of the parent.
This change was originally proposed by Sharlatan Hellseher: https://codeberg.org/guix/guix/pulls/7418#issuecomment-12239616 * zulip/integrations/bridge_with_irc/irc-mirror.py: Rename file to 'irc_mirror.py' to make it importable as a Python module. * zulip/integrations/bridge_with_irc/irc_mirror.py: Introduce `main' entry point so it can be used as a setuptools console script entry point. * zulip/setup.py: Register 'zulip-irc-bridge' as a console script so it's installed as part of the package output.
I've done what I could. I'm not familiar with Python, so I don't know what else to do. If you know how to fix the lint errors, you can force push before merging. Thanks! |
|
Sorry for the delay in getting back. Thanks for the updates. @laurynmm Could you please help assign an integration reviewer? The remaining mypy & gitlint fixes are listed below. Remaining Linting Update - Diffdiff --git a/zulip/integrations/bridge_with_irc/irc_mirror.py b/zulip/integrations/bridge_with_irc/irc_mirror.py
index 8d9bbee5..e0370986 100755
--- a/zulip/integrations/bridge_with_irc/irc_mirror.py
+++ b/zulip/integrations/bridge_with_irc/irc_mirror.py
@@ -46,7 +46,7 @@ def main() -> None:
options.client = "irc_mirror"
zulip_client = zulip.init_from_options(options)
try:
- from .irc_mirror_backend import IRCBot # type: ignore[import-untyped]
+ from .irc_mirror_backend import IRCBot # type: ignore[import-untyped]
except ImportError:
traceback.print_exc()
print(Commit messages - proposed replacements:Commit 1 - description: Commit 2 - title + description: |
Fixes: The IRC mirror times out when relaying IRC messages to Zulip due to SSL
socket sharing between parent and child processes after forking with
'mp.Process'.
How did you test this PR?
I've ran the mirror connected to a small IRC room and verified that the messages
where correctly relayed back and forth between the Zulip Cloud Guix instance and
the IRC room. I've used the following command:
Self-review checklist
(variable names, code reuse, readability, etc.).
Communicate decisions, questions, and potential concerns.
Individual commits are ready for review (see commit discipline).
Completed manual review and testing of the following: