Add helper base class for generating new replication endpoints #3632

erikjohnston · 2018-07-31T13:48:21Z

This will hopefully reduce the boiler plate required to implement new
internal HTTP requests.

This will hopefully reduce the boiler plate required to implement new internal HTTP requests.

…factor_repl_servlet

richvdh

I'm not entirely happy with the cleverness going on here, because it will make clicking through to implementations, type inference, etc not work properly.

Nevertheless it looks plausible modulo some nitpicking.

richvdh · 2018-08-07T15:09:30Z

synapse/replication/http/_base.py

+                        if e.code != 504 or not cls.RETRY_ON_TIMEOUT:
+                            raise
+
+                    logger.warn("send_federation_events_to_master request timed out")


could maybe use cls.NAME instead of send_federation_events_to_master

richvdh · 2018-08-07T15:18:39Z

synapse/replication/http/membership.py

-    Returns:
-        Deferred
+class ReplicationRemoteJoinRestServlet(ReplicationEndpoint):
+    """Does a remote join for the given user to the given room


please could you document the format of the URI - particularly since it seems to be changing.

(ditto below)

richvdh · 2018-08-07T15:49:53Z

synapse/replication/http/_base.py

+        PATH_ARGS (tuple[str]): A list of parameters to be added to the path.
+            Adding parameters to the path (rather than payload) can make it
+            easier to follow along in the log files.
+        POST (bool): True to use POST request with JSON body, or false to use


can we have METHOD (set to one of POST/GET/PUT etc) instead?

richvdh · 2018-08-07T15:51:54Z

synapse/replication/http/_base.py

+        http_server.register_paths(method, [pattern], handler)
+
+    def _cached_handler(self, request, txn_id, **kwargs):
+        """Wraps `_handle_request` the responses should be cached.


does whatnow?

erikjohnston · 2018-08-08T09:43:17Z

I'm not entirely happy with the cleverness going on here, because it will make clicking through to implementations, type inference, etc not work properly.

Yeah, I'm not entirely happy about it either, but I think its the step in the right direction in terms of avoiding explosions of custom implemented replication endpoints.

I had a quick play around trying to get some better click through/docs in vscode, but haven't had much luck (it doesn't seem to like inherited staticmethods....)

richvdh

can haz some words in the changelog pls?

otherwise lgtm.

richvdh · 2018-08-08T22:07:06Z

synapse/replication/http/_base.py

@@ -190,7 +190,9 @@ def register(self, http_server):
        http_server.register_paths(method, [pattern], handler)

    def _cached_handler(self, request, txn_id, **kwargs):
-        """Wraps `_handle_request` the responses should be cached.
+        """Called on new incoming requests when caching is enabled. Checks
+        if their is a cached response for the request and returns that,


richvdh · 2018-08-08T22:11:26Z

synapse/replication/http/_base.py

@@ -174,8 +185,7 @@ def register(self, http_server):
        url_args = list(self.PATH_ARGS)
        method = "GET"


this is dead

richvdh · 2018-08-08T22:13:05Z

synapse/replication/http/_base.py

+
+    def _cached_handler(self, request, txn_id, **kwargs):
+        """Called on new incoming requests when caching is enabled. Checks
+        if their is a cached response for the request and returns that,


@vojeroen

Features -------- - Add support for the SNI extension to federation TLS connections. Thanks to @vojeroen! ([\#3439](#3439)) - Add /_media/r0/config ([\#3184](#3184)) - speed up /members API and add `at` and `membership` params as per MSC1227 ([\#3568](#3568)) - implement `summary` block in /sync response as per MSC688 ([\#3574](#3574)) - Add lazy-loading support to /messages as per MSC1227 ([\#3589](#3589)) - Add ability to limit number of monthly active users on the server ([\#3633](#3633)) - Support more federation endpoints on workers ([\#3653](#3653)) - Basic support for room versioning ([\#3654](#3654)) - Ability to disable client/server Synapse via conf toggle ([\#3655](#3655)) - Ability to whitelist specific threepids against monthly active user limiting ([\#3662](#3662)) - Add some metrics for the appservice and federation event sending loops ([\#3664](#3664)) - Where server is disabled, block ability for locked out users to read new messages ([\#3670](#3670)) - set admin uri via config, to be used in error messages where the user should contact the administrator ([\#3687](#3687)) - Synapse's presence functionality can now be disabled with the "use_presence" configuration option. ([\#3694](#3694)) - For resource limit blocked users, prevent writing into rooms ([\#3708](#3708)) Bugfixes -------- - Fix occasional glitches in the synapse_event_persisted_position metric ([\#3658](#3658)) - Fix bug on deleting 3pid when using identity servers that don't support unbind API ([\#3661](#3661)) - Make the tests pass on Twisted < 18.7.0 ([\#3676](#3676)) - Don’t ship recaptcha_ajax.js, use it directly from Google ([\#3677](#3677)) - Fixes test_reap_monthly_active_users so it passes under postgres ([\#3681](#3681)) - Fix mau blocking calulation bug on login ([\#3689](#3689)) - Fix missing yield in synapse.storage.monthly_active_users.initialise_reserved_users ([\#3692](#3692)) - Improve HTTP request logging to include all requests ([\#3700](#3700)) - Avoid timing out requests while we are streaming back the response ([\#3701](#3701)) - Support more federation endpoints on workers ([\#3705](#3705), [\#3713](#3713)) - Fix "Starting db txn 'get_all_updated_receipts' from sentinel context" warning ([\#3710](#3710)) - Fix bug where `state_cache` cache factor ignored environment variables ([\#3719](#3719)) - Fix bug in v0.33.3rc1 which caused infinite loops and OOMs ([\#3723](#3723)) - Fix bug introduced in v0.33.3rc1 which made the ToS give a 500 error ([\#3732](#3732)) Deprecations and Removals ------------------------- - The Shared-Secret registration method of the legacy v1/register REST endpoint has been removed. For a replacement, please see [the admin/register API documentation](https://github.com/matrix-org/synapse/blob/master/docs/admin_api/register_api.rst). ([\#3703](#3703)) Internal Changes ---------------- - The test suite now can run under PostgreSQL. ([\#3423](#3423)) - Refactor HTTP replication endpoints to reduce code duplication ([\#3632](#3632)) - Tests now correctly execute on Python 3. ([\#3647](#3647)) - Sytests can now be run inside a Docker container. ([\#3660](#3660)) - Port over enough to Python 3 to allow the sytests to start. ([\#3668](#3668)) - Update docker base image from alpine 3.7 to 3.8. ([\#3669](#3669)) - Rename synapse.util.async to synapse.util.async_helpers to mitigate async becoming a keyword on Python 3.7. ([\#3678](#3678)) - Synapse's tests are now formatted with the black autoformatter. ([\#3679](#3679)) - Implemented a new testing base class to reduce test boilerplate. ([\#3684](#3684)) - Rename MAU prometheus metrics ([\#3690](#3690)) - add new error type ResourceLimit ([\#3707](#3707)) - Logcontexts for replication command handlers ([\#3709](#3709)) - Update admin register API documentation to reference a real user ID. ([\#3712](#3712))

erikjohnston added 3 commits July 31, 2018 14:32

Add helper base class for generating new replication endpoints

d81602b

This will hopefully reduce the boiler plate required to implement new internal HTTP requests.

Use new helper base class for ReplicationSendEventRestServlet

729b672

Use new helper base class for membership requests

443da00

erikjohnston requested a review from a team July 31, 2018 13:48

erikjohnston added 3 commits July 31, 2018 14:48

Newsfile

16bd63f

Merge branch 'develop' of github.com:matrix-org/synapse into erikj/re…

cb298ff

…factor_repl_servlet

Fix isort

051a99c

This was referenced Aug 6, 2018

Move more federation APIs to workers #3653

Merged

Allow profile updates to happen on workers #3659

Merged

richvdh reviewed Aug 7, 2018

View reviewed changes

erikjohnston added 2 commits August 8, 2018 10:29

Fixup logging and docstrings

5011417

Rename POST param to METHOD

bebe325

erikjohnston assigned richvdh Aug 8, 2018

richvdh approved these changes Aug 8, 2018

View reviewed changes

richvdh reviewed Aug 8, 2018

View reviewed changes

richvdh assigned erikjohnston and unassigned richvdh Aug 8, 2018

Fixup wording and remove dead code

6256479

erikjohnston merged commit 2bdafaf into develop Aug 9, 2018

erikjohnston deleted the erikj/refactor_repl_servlet branch September 20, 2018 13:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add helper base class for generating new replication endpoints #3632

Add helper base class for generating new replication endpoints #3632

erikjohnston commented Jul 31, 2018

richvdh left a comment

richvdh Aug 7, 2018

richvdh Aug 7, 2018

richvdh Aug 7, 2018

richvdh Aug 7, 2018

richvdh Aug 7, 2018

erikjohnston commented Aug 8, 2018

richvdh left a comment

richvdh Aug 8, 2018

richvdh Aug 8, 2018

richvdh Aug 8, 2018

		@@ -174,8 +185,7 @@ def register(self, http_server):
		url_args = list(self.PATH_ARGS)
		method = "GET"

Add helper base class for generating new replication endpoints #3632

Add helper base class for generating new replication endpoints #3632

Conversation

erikjohnston commented Jul 31, 2018

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikjohnston commented Aug 8, 2018

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment