Option to replicate user profiles to another server #3112
Conversation
| } for r in batch_rows | ||
| } | ||
|
|
||
| url = "https://%s/_matrix/federation/v1/replicate_profiles" % (host,) |
There was a problem hiding this comment.
There doesn't seem to be a handler for this. Are profiles intended to be replicated to a non-homeserver?
There was a problem hiding this comment.
correct; this is an experiment in replicating user profiles over to the IS so that for private IS deployments the IS can act as a global address book.
and same for the profile replication
| @@ -108,6 +112,10 @@ def default_config(self, **kwargs): | |||
| - vector.im | |||
| - riot.im | |||
|
|
|||
| # If enabled, user IDs, display names and avatar URLs will be replicated | |||
| # to this server whenever they change. | |||
There was a problem hiding this comment.
we need to say that this is an experimental addition to the identity server API for supporting global user directories, and that the targets here should be identity server hostnames (otherwise folks will assume it's for HSes)
There was a problem hiding this comment.
Done, although it's in the /_matrix/federation path so I've said it's an API that's implemented by sydent rather than part of the IS API as such.
synapse/handlers/profile.py
Outdated
| if len(self.hs.config.replicate_user_profiles_to) > 0: | ||
| reactor.callWhenRunning(self._assign_profile_replication_batches) | ||
| reactor.callWhenRunning(self._replicate_profiles) | ||
| self.clock.looping_call( |
There was a problem hiding this comment.
It'd be good to spell out in a comment why we're looping _replicate_profiles. I can see we'll need to do a catchup job at launch, but why do we then keep running it every 2 minutes, given whenever we set profile data it kicks off a sync anyway?
| logger.info("Assigned %d profile batch numbers", total) | ||
|
|
||
| @defer.inlineCallbacks | ||
| def _replicate_profiles(self): |
There was a problem hiding this comment.
How does this handle overlapping calls? (e.g. two _replicate_profiles runs being kicked off by two users setting profile data at the same time, or colliding with the looping syncer)?
There was a problem hiding this comment.
They'll get different batch numbers depending on which one got serviced first, then probably the first call to _replicate_profiles will send both batches and the second will do nothing. If it collides with the looping syncer it'll depend which db txn gets done first.
|
this looks very plausible - lgtm mod comments. |
synapse/handlers/profile.py
Outdated
| signed_body = sign_json(body, self.hs.hostname, self.hs.config.signing_key[0]) | ||
| try: | ||
| yield self.http_client.post_json_get_json(url, signed_body) | ||
| self.store.update_replication_batch_for_host(host, batchnum) |
synapse/storage/profile.py
Outdated
| txn.execute("SELECT MAX(batch) as maxbatch FROM profiles") | ||
| rows = self.cursor_to_dict(txn) | ||
| return rows[0]['maxbatch'] | ||
| max_batch = yield self.runInteraction( |
There was a problem hiding this comment.
you could just return self.runInteraction(...) rather than using inlineCallbacks
|
|
||
| CREATE INDEX profiles_batch_idx ON profiles(batch); | ||
|
|
||
| CREATE TABLE profile_replication_status ( |
There was a problem hiding this comment.
please can we get into the habit of commenting our schema files to describe what the tables and the columns therein do.
tests/handlers/test_profile.py
Outdated
| @@ -75,7 +75,7 @@ def register_query_handler(query_type, handler): | |||
| @defer.inlineCallbacks | |||
| def test_get_my_name(self): | |||
| yield self.store.set_profile_displayname( | |||
| self.frank.localpart, "Frank" | |||
| self.frank.localpart, "Frank", 1 | |||
There was a problem hiding this comment.
it'd be nice to add trailing commas while you're here
There was a problem hiding this comment.
ah yep, done (plus the others)
Unnecessary inlineCallbacks, missing yield, SQL comments & trailing commas.
No description provided.