interrealm: Add option to run the bridge unidirectionally.

[rht]

This is for the use case of playground.zulipdev.org.

[zulipbot]

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.

[timabbott]

@neiljp can you take a look at this?

[timabbott]

Can we do this without duplicating code between the two paths?

[rht]

This is the simplest way to do it without having to add more lines.

[neiljp]

I'm not sure if this is cleaner to read, but if this is equivalent then you could have:

    1 if not args.oneway:
    2     pipe_event2 = ...
    3     p2 = mp.Process(...)
    4     p2.start()
    5 p1.start()
    6 print("Listening")
    7 p1.join()
    8 if not args.oneway
    9     p2.join()

Does the start() order matter?

[rht]

Yeah, it is possible to do it that way too.

[neiljp]

Perhaps this could be achieved more cleanly by using concurrent.futures?

[neiljp]

I'm not sure if this is cleaner to read, but if this is equivalent then you could have:

    1 if not args.oneway:
    2     pipe_event2 = ...
    3     p2 = mp.Process(...)
    4     p2.start()
    5 p1.start()
    6 print("Listening")
    7 p1.join()
    8 if not args.oneway
    9     p2.join()

Does the start() order matter?

[rht]

@neiljp how does a concurrent.futures version look like? If it is cleaner, maybe it should be used in the other bridges.

[neiljp]

@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.

[rht]

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.

[neiljp]

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.

[rht]

"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.

[neiljp]

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.

[zulipbot]

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.