Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Implement RoomMemberWorkerHandler #2992

Merged
merged 4 commits into from
Mar 14, 2018
Merged

Conversation

erikjohnston
Copy link
Member

No description provided.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm going to have to insist on RoomMemberWorkerHandler going into a different file. Importing the synapse.replication.http symbols makes it very confusing otherwise.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks generally plausible, but a few questions

@@ -15,6 +15,7 @@


import send_event
import membership
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from synapse.replication.http import membership, surely?

from synapse.types import Requester, UserID
from synapse.util.distributor import user_left_room, user_joined_room

import logging
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

system libs first please

port=self.config.worker_replication_http_port,
user_id=target.to_string(),
room_id=room_id,
change="join",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why "join" and "left", rather than "joined" and "left", or "join" and "leave"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I had it in my head that it was user_join_room. Will change.

requester = Requester.deserialize(self.store, content["requester"])

if requester.user:
request.authenticated_entity = requester.user.to_string()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, why are we doing this, given we don't appear to ever use request again?

Copy link
Member Author

@erikjohnston erikjohnston Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its used to log the authenticated entity during Processed request... logging

@erikjohnston erikjohnston force-pushed the erikj/implement_member_workre branch from 7631189 to 62ad701 Compare March 14, 2018 14:17
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@erikjohnston erikjohnston merged commit 57db62e into develop Mar 14, 2018
@erikjohnston erikjohnston deleted the erikj/implement_member_workre branch March 22, 2018 13:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants