/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add get endpoint for pushers #716

Merged
merged 7 commits into from Apr 12, 2016

Conversation

Reviewers
No reviews
Assignees

@erikjohnston erikjohnston

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@dbkr @erikjohnston
@dbkr
Member

dbkr commented Apr 11, 2016

As per matrix-org/matrix-doc#308
Tested by matrix-org/sytest#227
@dbkr
Add get endpoint for pushers 
As per matrix-org/matrix-doc#308

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #383 origin/dbkr/get_pushers succeeded in 29 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #374 origin/dbkr/get_pushers succeeded in 5 min 50 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #379 origin/dbkr/get_pushers succeeded in 4 min 41 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #427 origin/dbkr/get_pushers succeeded in 1 min 20 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
8a76094

@dbkr dbkr assigned erikjohnston Apr 11, 2016

@dbkr dbkr referenced this pull request in matrix-org/sytest Apr 11, 2016

Merged

Test for getting a user's pushers #227

@erikjohnston erikjohnston and 1 other commented on an outdated diff Apr 12, 2016

View full outdated diff
synapse/rest/client/v1/pusher.py
@@ -27,14 +27,47 @@
class PusherRestServlet(ClientV1RestServlet):
- PATTERNS = client_path_patterns("/pushers/set$")
+ PATTERNS = client_path_patterns("/pushers(/set)?$")
@erikjohnston

erikjohnston Apr 12, 2016

Owner

Would probably be clearer if this was split into two, since /pushers is only valid for GET and /pushers/set is only valid for POST

@dbkr

dbkr Apr 12, 2016

Member

Done. Was debating which was neater but I think you're right.

@erikjohnston erikjohnston commented on an outdated diff Apr 12, 2016

View full outdated diff
synapse/storage/pusher.py
@@ -76,6 +76,25 @@ def r(txn):
defer.returnValue(rows)
@defer.inlineCallbacks
+ def get_pushers_by_app_user_id(self, user_id):
+ def r(txn):
+ sql = (
+ "SELECT * FROM pushers"
+ " WHERE user_name = ?"
+ )
+
+ txn.execute(sql, (user_id,))
+ rows = self.cursor_to_dict(txn)
+
+ return self._decode_pushers_rows(rows)
@erikjohnston

erikjohnston Apr 12, 2016

Owner

Generally I prefer to do all CPU work on the main thread, to avoid taking out database threads. Might also be nice to use the _simple_select_list func.

This was referenced Apr 12, 2016

Merged
Merged

dbkr added some commits Apr 12, 2016

@dbkr
Split into separate servlet classes

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #391 origin/dbkr/get_pushers succeeded in 32 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #382 origin/dbkr/get_pushers succeeded in 4 min 51 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #387 origin/dbkr/get_pushers succeeded in 5 min 1 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #435 origin/dbkr/get_pushers succeeded in 1 min 11 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
d937f34
@dbkr
Mis-named function
7b39bcd
@dbkr
Tidy up get_pusher functions 
Decodes pushers rows on the main thread rather than the db thread and uses _simple_select_list. Also do the same to the function I copied and factor out the duplication into a helper function.

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #392 origin/dbkr/get_pushers succeeded in 29 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #383 origin/dbkr/get_pushers succeeded in 5 min 49 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #388 origin/dbkr/get_pushers succeeded in 4 min 38 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #436 origin/dbkr/get_pushers succeeded in 1 min 21 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
44891b4
@dbkr
Member

dbkr commented Apr 12, 2016

ptal

@erikjohnston erikjohnston commented on an outdated diff Apr 12, 2016

View full outdated diff
synapse/storage/pusher.py
+ "id",
+ "user_name",
+ "access_token",
+ "profile_tag",
+ "kind",
+ "app_id",
+ "app_display_name",
+ "device_display_name",
+ "pushkey",
+ "ts",
+ "lang",
+ "data",
+ "last_stream_ordering",
+ "last_success",
+ "failing_since",
+
@erikjohnston

erikjohnston Apr 12, 2016

Owner

Spurious blank line?

@erikjohnston erikjohnston commented on an outdated diff Apr 12, 2016

View full outdated diff
synapse/storage/pusher.py
+ "user_name",
+ "access_token",
+ "profile_tag",
+ "kind",
+ "app_id",
+ "app_display_name",
+ "device_display_name",
+ "pushkey",
+ "ts",
+ "lang",
+ "data",
+ "last_stream_ordering",
+ "last_success",
+ "failing_since",
+
+ ]
@erikjohnston

erikjohnston Apr 12, 2016

Owner

Please include a desc="get_pushers_by" for monitoring purposes :)

dbkr added some commits Apr 12, 2016

@dbkr
Make the /set part mandatory

All checks have passed

7 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #393 origin/dbkr/get_pushers succeeded in 27 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #384 origin/dbkr/get_pushers succeeded in 5 min 33 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #389 origin/dbkr/get_pushers succeeded in 4 min 29 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
a04c076
@dbkr
Oops, forgot the desc.

All checks have passed

5 successful checks
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #437 origin/dbkr/get_pushers succeeded in 1 min 11 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
c1267d0
@dbkr
Unneccessarywhitespaceisunnecessary

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #394 origin/dbkr/get_pushers succeeded in 28 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #385 origin/dbkr/get_pushers succeeded in 5 min 49 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #390 origin/dbkr/get_pushers succeeded in 4 min 17 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #438 origin/dbkr/get_pushers succeeded in 1 min 9 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
7984ffd
@erikjohnston
Owner

erikjohnston commented Apr 12, 2016

LGTM

@dbkr dbkr merged commit d33d623 into develop Apr 12, 2016
8 checks passed

8 checks passed

Flake8 + Packaging (Commit) Build #394 origin/dbkr/get_pushers succeeded in 28 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #385 origin/dbkr/get_pushers succeeded in 5 min 49 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #390 origin/dbkr/get_pushers succeeded in 4 min 17 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #438 origin/dbkr/get_pushers succeeded in 1 min 9 sec
Details
Unit Tests (Merged PR) Build finished.
Details

@richvdh richvdh deleted the dbkr/get_pushers branch Dec 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment