Convert appservice to async. #7973

clokep · 2020-07-28T17:07:54Z

Converts synapse.appservice package to async / await.

This was fairly straightforward, minus the tests which needed quite a bit of work.

clokep · 2020-07-28T18:50:11Z

synapse/appservice/scheduler.py

        logger.info("Starting retries on %s", self.service.id)
        try:
            while True:
-                txn = yield self.store.get_oldest_unsent_txn(self.service)
+                txn = await self.store.get_oldest_unsent_txn(self.service)
                if not txn:
                    # nothing left: we're done!
                    self.callback(self)


As far as I can tell this self.callback is _TransactionController.on_recovered, which used to be a inlineCallbacks (is now async), but doesn't get awaited on. This seems wrong.

Note that this class is constructed here.

Looks like this started as a normal function and then turned into a deferred: f260cb7#diff-b1805daad13332892217490e8c31cc0cL169-L171 without the call being modified: f260cb7#diff-b1805daad13332892217490e8c31cc0cL231

I added an await here. 👍

anoadragon453

I see what you mean about the tests :) But while verbose, they all make sense.

Just a few things.

tests/appservice/test_appservice.py

anoadragon453 · 2020-07-28T23:44:13Z

synapse/appservice/api.py

@@ -128,8 +125,7 @@ def query_alias(self, service, alias):
            logger.warning("query_alias to %s threw exception %s", uri, ex)


Hmm, the:

if e.code == 404: return False

bit here is also useless.

Why is that useless?

I'm unsure if you're asking me to make a change.

Sorry, I had noticed you removed the response = None line above as it wasn't necessary.

I'd say it's useless as if that if statement doesn't trigger, then the function will return False after the try statement anyways.

Though it may've been that returning True if code != 404 was the intention here.

synapse/appservice/api.py

synapse/handlers/appservice.py

anoadragon453 · 2020-07-28T23:54:41Z

synapse/appservice/scheduler.py

        logger.info("Starting retries on %s", self.service.id)
        try:
            while True:
-                txn = yield self.store.get_oldest_unsent_txn(self.service)
+                txn = await self.store.get_oldest_unsent_txn(self.service)
                if not txn:
                    # nothing left: we're done!
                    self.callback(self)


Looks like this started as a normal function and then turned into a deferred: f260cb7#diff-b1805daad13332892217490e8c31cc0cL169-L171 without the call being modified: f260cb7#diff-b1805daad13332892217490e8c31cc0cL231

clokep · 2020-07-29T11:24:11Z

@anoadragon453 I handled or replied to all of your comments. Let me know if there are more changes to make!

anoadragon453

Looks good! The if e.code == 404 was just a minor cleanup thing, but shouldn't block this PR.

…bership_join_count * 'develop' of github.com:matrix-org/synapse: Update workers docs (#7990) Fix invite rejection when we have no forward-extremeties (#7980) Fix typo in docs/workers.md (#7992) Convert federation client to async/await. (#7975) Convert appservice to async. (#7973) Convert some of the data store to async. (#7976) Fix formatting of changelog and upgrade notes Ensure that remove_pusher is always async (#7981) Add deprecation warnings 1.18.0 Update worker docs with recent enhancements (#7969) Ensure the msg property of HttpResponseException is a string. (#7979) Remove from the event_relations table when purging historical events. (#7978) Add additional logging for SAML sessions. (#7971) Add MSC reference to changelog for #7736 Re-implement unread counts (#7736) Various improvements to the docs (#7899)

@lugino-emeritus

Synapse 1.19.0rc1 (2020-08-13) ============================== Removal warning --------------- As outlined in the [previous release](https://github.com/matrix-org/synapse/releases/tag/v1.18.0), we are no longer publishing Docker images with the `-py3` tag suffix. On top of that, we have also removed the `latest-py3` tag. Please see [the announcement in the upgrade notes for 1.18.0](https://github.com/matrix-org/synapse/blob/develop/UPGRADE.rst#upgrading-to-v1180). Features -------- - Add option to allow server admins to join rooms which fail complexity checks. Contributed by @lugino-emeritus. ([\#7902](#7902)) - Add an option to purge room or not with delete room admin endpoint (`POST /_synapse/admin/v1/rooms/<room_id>/delete`). Contributed by @dklimpel. ([\#7964](#7964)) - Add rate limiting to users joining rooms. ([\#8008](#8008)) - Add a `/health` endpoint to every configured HTTP listener that can be used as a health check endpoint by load balancers. ([\#8048](#8048)) - Allow login to be blocked based on the values of SAML attributes. ([\#8052](#8052)) - Allow guest access to the `GET /_matrix/client/r0/rooms/{room_id}/members` endpoint, according to MSC2689. Contributed by Awesome Technologies Innovationslabor GmbH. ([\#7314](#7314)) Bugfixes -------- - Fix a bug introduced in Synapse v1.7.2 which caused inaccurate membership counts in the room directory. ([\#7977](#7977)) - Fix a long standing bug: 'Duplicate key value violates unique constraint "event_relations_id"' when message retention is configured. ([\#7978](#7978)) - Fix "no create event in auth events" when trying to reject invitation after inviter leaves. Bug introduced in Synapse v1.10.0. ([\#7980](#7980)) - Fix various comments and minor discrepencies in server notices code. ([\#7996](#7996)) - Fix a long standing bug where HTTP HEAD requests resulted in a 400 error. ([\#7999](#7999)) - Fix a long-standing bug which caused two copies of some log lines to be written when synctl was used along with a MemoryHandler logger. ([\#8011](#8011), [\#8012](#8012)) Updates to the Docker image --------------------------- - We no longer publish Docker images with the `-py3` tag suffix, as [announced in the upgrade notes](https://github.com/matrix-org/synapse/blob/develop/UPGRADE.rst#upgrading-to-v1180). ([\#8056](#8056)) Improved Documentation ---------------------- - Document how to set up a client .well-known file and fix several pieces of outdated documentation. ([\#7899](#7899)) - Improve workers docs. ([\#7990](#7990), [\#8000](#8000)) - Fix typo in `docs/workers.md`. ([\#7992](#7992)) - Add documentation for how to undo a room shutdown. ([\#7998](#7998), [\#8010](#8010)) Internal Changes ---------------- - Reduce the amount of whitespace in JSON stored and sent in responses. Contributed by David Vo. ([\#7372](#7372)) - Switch to the JSON implementation from the standard library and bump the minimum version of the canonicaljson library to 1.2.0. ([\#7936](#7936), [\#7979](#7979)) - Convert various parts of the codebase to async/await. ([\#7947](#7947), [\#7948](#7948), [\#7949](#7949), [\#7951](#7951), [\#7963](#7963), [\#7973](#7973), [\#7975](#7975), [\#7976](#7976), [\#7981](#7981), [\#7987](#7987), [\#7989](#7989), [\#8003](#8003), [\#8014](#8014), [\#8016](#8016), [\#8027](#8027), [\#8031](#8031), [\#8032](#8032), [\#8035](#8035), [\#8042](#8042), [\#8044](#8044), [\#8045](#8045), [\#8061](#8061), [\#8062](#8062), [\#8063](#8063), [\#8066](#8066), [\#8069](#8069), [\#8070](#8070)) - Move some database-related log lines from the default logger to the database/transaction loggers. ([\#7952](#7952)) - Add a script to detect source code files using non-unix line terminators. ([\#7965](#7965), [\#7970](#7970)) - Log the SAML session ID during creation. ([\#7971](#7971)) - Implement new experimental push rules for some users. ([\#7997](#7997)) - Remove redundant and unreliable signature check for v1 Identity Service lookup responses. ([\#8001](#8001)) - Improve the performance of the register endpoint. ([\#8009](#8009)) - Reduce less useful output in the newsfragment CI step. Add a link to the changelog section of the contributing guide on error. ([\#8024](#8024)) - Rename storage layer objects to be more sensible. ([\#8033](#8033)) - Change the default log config to reduce disk I/O and storage for new servers. ([\#8040](#8040)) - Add an assertion on `prev_events` in `create_new_client_event`. ([\#8041](#8041)) - Add a comment to `ServerContextFactory` about the use of `SSLv23_METHOD`. ([\#8043](#8043)) - Log `OPTIONS` requests at `DEBUG` rather than `INFO` level to reduce amount logged at `INFO`. ([\#8049](#8049)) - Reduce amount of outbound request logging at `INFO` level. ([\#8050](#8050)) - It is no longer necessary to explicitly define `filters` in the logging configuration. (Continuing to do so is redundant but harmless.) ([\#8051](#8051)) - Add and improve type hints. ([\#8058](#8058), [\#8064](#8064), [\#8060](#8060), [\#8067](#8067))

* commit '4cce8ef74': Convert appservice to async. (#7973)

clokep added 8 commits July 28, 2020 10:14

Convert appservice to async.

70e9a40

Convert appservice API.

2b8bcea

Convert scheduler start.

08afe80

Convert _ServiceQueuer

118f2f7

Convert _Recoverer

15d4ba1

Finish conversion.

99a0ec1

Return proper type in tests.

957313d

Add changelog.

c55d9cb

clokep commented Jul 28, 2020

View reviewed changes

clokep requested a review from a team July 28, 2020 18:50

anoadragon453 suggested changes Jul 28, 2020

View reviewed changes

clokep added 3 commits July 29, 2020 07:17

Update comments.

49d09f9

Fix use of log_failure.

99e3658

Await the callback.

5d7fdf6

clokep requested a review from anoadragon453 July 29, 2020 11:23

anoadragon453 approved these changes Jul 29, 2020

View reviewed changes

clokep merged commit 4cce8ef into develop Jul 30, 2020

clokep deleted the clokep/async-appservice branch July 30, 2020 11:27

clokep mentioned this pull request Jul 30, 2020

Convert inlineCallbacks to async/await #7988

Closed

48 tasks

babolivier pushed a commit that referenced this pull request Sep 1, 2021

Merge commit '4cce8ef74' into anoa/dinsic_release_1_21_x

6d02e38

* commit '4cce8ef74': Convert appservice to async. (#7973)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert appservice to async. #7973

Convert appservice to async. #7973

clokep commented Jul 28, 2020

clokep Jul 28, 2020

anoadragon453 Jul 28, 2020

clokep Jul 29, 2020

anoadragon453 left a comment

anoadragon453 Jul 28, 2020

clokep Jul 29, 2020

anoadragon453 Jul 29, 2020

anoadragon453 Jul 28, 2020

clokep commented Jul 29, 2020

anoadragon453 left a comment

		@@ -128,8 +125,7 @@ def query_alias(self, service, alias):
		logger.warning("query_alias to %s threw exception %s", uri, ex)

Convert appservice to async. #7973

Convert appservice to async. #7973

Conversation

clokep commented Jul 28, 2020

clokep Jul 28, 2020

Choose a reason for hiding this comment

anoadragon453 Jul 28, 2020

Choose a reason for hiding this comment

clokep Jul 29, 2020

Choose a reason for hiding this comment

anoadragon453 left a comment

Choose a reason for hiding this comment

anoadragon453 Jul 28, 2020

Choose a reason for hiding this comment

clokep Jul 29, 2020

Choose a reason for hiding this comment

anoadragon453 Jul 29, 2020

Choose a reason for hiding this comment

anoadragon453 Jul 28, 2020

Choose a reason for hiding this comment

clokep commented Jul 29, 2020

anoadragon453 left a comment

Choose a reason for hiding this comment