WIP: Use threads to allow more than one connection #140
Conversation
yggdr
force-pushed
the
feature/threaded_accept
branch
from
3fcb57e
to
abd410a
Compare
|
@jettify ? |
| def __init__(self, | ||
| client: socket.socket, | ||
| addr: Tuple[str, int], | ||
| monitor: Monitor |
There was a problem hiding this comment.
I think instead of sharing full monitor object safer to pass required arguments like thread_id/etc, less sharing better
There was a problem hiding this comment.
Agreed, I had planned on cleaning this up, but for the first impression, I just wanted to make it work.
| self._console_future.result() | ||
| console_proxy(self._sin, self._sout, self._host, self._console_port) | ||
| if self._monitor._console_future is not None: | ||
| self._monitor._console_future.result() |
There was a problem hiding this comment.
I am wondering, if we can wait for console server to start when we init it instead of here. This should simplify things a bit.
There was a problem hiding this comment.
I'm not sure I understand what you mean. Could you elaborate?
There was a problem hiding this comment.
I just thinking if it is better to move _console_future.result() closer where we actually create this future object.
aiomonitor/aiomonitor/monitor.py
Lines 108 to 109 in 230a351
In current implementation we create future and wait it once we need to be sure that python console is started, instead doing this should we just wait console immediately?
There was a problem hiding this comment.
reworked this part of code in master
| sin = client.makefile('r', encoding='utf-8') | ||
| self._interactive_loop(sin, sout) | ||
| except (socket.timeout, OSError): | ||
| threading.Thread(target=CLI(client, addr, self)).start() |
There was a problem hiding this comment.
Should we make this tread demonized?
There was a problem hiding this comment.
That's a very good idea.
|
I like the idea, do not see any big issues with threads. |
|
This will be resolved by backporting aiomonitor-ng. 😉 |
I'd like to have the possibility to connect more than once to a monitor, so I tried a threading approach.
This is still work in progress, I'd like someone who knows more about the gritty details of threads to have a look here. It seems to work well, but who knows what kind of bugs might lurk in the details.