python-zulip-api
python-zulip-api copied to clipboard
interrealm: Add option to run the bridge unidirectionally.
This is for the use case of playground.zulipdev.org.
Heads up @rht, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master
branch and resolve your pull request's merge conflicts accordingly.
@neiljp can you take a look at this?
@neiljp how does a concurrent.futures
version look like? If it is cleaner, maybe it should be used in the other bridges.
@rht I haven't explored it in detail for this application, but it could perhaps be something like the following (untested):
from concurrent.futures import ProcessPoolExecutor, wait
pipe_event1 = create_pipe_event(client2, bot1, bot2, args.stream)
with ProcessPoolExecutor(max_workers=2) as executor:
f = [executor.submit(client1.call_on_each_event, pipe_event1, ["message"])]
if not args.oneway:
pipe_event2 = create_pipe_event(client1, bot2, bot1, args.stream)
f += [executor.submit(client2.call_on_each_event, pipe_event2, ["message"])]
print("Listening...")
wait(f)
I used this module in zulip-terminal recently.
That looks like a bit longer than I thought (concurrent.futures
as a common interface to multiprocessing
and threading
). Would it be possible to do
f1 = executor.submit(client1.call_on_each_event, pipe_event1, ["message"])
f1.future()
?
Even if this is possible, there is still create_pipe_event
construct, which is another extra structure.
Longer? The original is 13 lines, and the new one is 8, plus the import line - plus cleaner, in my opinion, though no guarantee it's directly equivalent as I'm not completely familiar with the existing code.
Re your question, f1
is already a future; in my draft code above, the futures are just being used to test the completion state of the code, which by my understanding is equivalent to a join
. I found the library docs quite readable - assuming I understand them, of course ;)
Refactoring for the other structures is a separate step, which I hadn't considered.
"Longer" in a horizontal way rather than vertical way, in terms of the number of nested expressions.
(Also, I somehow forgot the fact that create_pipe_event
was a construct in the bridge itself.)
Also, typo: f1.result()
instead of f1.future()
.
Aside: maybe they (anything that runs n functions in parallel) could be wrapped in a run_parallel
so that no one has to see the gory detail of the parallelization?
run_parallel((func1, args1), (func2, args2), ...)
There is a run_parallel
function in zulip/zulip
, but it uses os
process forks.
The extra level of nesting is due to that context manager, which tidies things up automatically - does the same happen currently?
You could get a result by f1.result()
, but currently the result of call_on_each_event
would be None
, of course; I think it would probably block at that point, too.
Heads up @rht, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master
branch and resolve your pull request's merge conflicts accordingly.