Split RoomMemberHandler into base and master class #2987
Conversation
The intention here is to split the class into the bits that can be done on workers and the bits that have to be done on the master. In future there will also be a class that can be run on the worker, which will delegate work to the master when necessary.
I added yields when calling user_left_room, but they shouldn't matter on the master process as they always return None anyway.
synapse/handlers/room_member.py
Outdated
| @@ -31,6 +28,10 @@ | |||
| from synapse.util.async import Linearizer | |||
| from synapse.util.distributor import user_left_room, user_joined_room | |||
|
|
|||
| import abc | |||
There was a problem hiding this comment.
https://github.com/matrix-org/synapse/blob/master/docs/code_style.rst:
As per PEP-8, imports should be grouped in the following order, with a blank line between each group:
standard library imports
related third party imports
local application/library specific imports
this is going in the opposite direction!
There was a problem hiding this comment.
Hmm. I thought we always put e.g. import logging last tbh. (Doesn't your IDE rearrange imports into that order?)
synapse/handlers/room_member.py
Outdated
| self.distributor = hs.get_distributor() | ||
| self.distributor.declare("user_joined_room") | ||
| self.distributor.declare("user_left_room") | ||
| self.http_client = hs.get_simple_http_client() |
There was a problem hiding this comment.
This needs to be removed as its spurious
synapse/handlers/room_member.py
Outdated
|
|
||
| @abc.abstractmethod | ||
| def _user_left_room(self, target, room_id): | ||
| raise NotImplementedError() |
There was a problem hiding this comment.
These need to be docced.
erikjohnston
force-pushed
the
erikj/split_room_member_handler
branch
from
b6c2b41 to
82f16fa
Compare
| ) | ||
| defer.returnValue({}) | ||
|
|
||
| def get_or_register_3pid_guest(self, requester, medium, address, inviter_user_id): |
There was a problem hiding this comment.
is there a reason to pass requester here?
There was a problem hiding this comment.
Yup, as we'll need to hit out to the master and we want to include the Requester in the logging there.
There was a problem hiding this comment.
(We don't do it for the user_*_room as that just goes straight into the distributor which immediately strips the context)
synapse/handlers/room_member.py
Outdated
| """Try and join a room that this server is not in | ||
|
|
||
| Args: | ||
| remote_room_hosts (list[str]): List of servers that can be used |
There was a problem hiding this comment.
likewise for some of the other methods
The intention here is to split the class into the bits that can be done
on workers and the bits that have to be done on the master.
In future there will also be a class that can be run on the worker,
which will delegate work to the master when necessary.