Add GET media/v1/config #3184

Half-Shot · 2018-05-03T12:57:39Z

Part of matrix-org/matrix-spec-proposals#1189.
Exposes the /media/r0/config endpoint

matrixbot · 2018-05-03T12:57:41Z

Can one of the admins verify this patch?

matrixbot · 2018-05-03T12:57:41Z

Can one of the admins verify this patch?

erikjohnston · 2018-05-04T15:28:13Z

synapse/rest/media/v1/limits_resource.py

+
+
+class MediaLimitsResource(Resource):
+    # isLeaf = True


Why is this commented out?

Oh. I had no idea what it was for and forgot to ask. So errm, what is it for?

https://stackoverflow.com/a/42799031

erikjohnston · 2018-05-04T15:29:30Z

synapse/rest/media/v1/limits_resource.py

+        Resource.__init__(self)
+        self.limits_dict = {}
+        config = hs.get_config()
+        self.limits_dict["upload_size"] = config.max_upload_size


I'd write this as:

def __init__(self, hs): Resource.__init__(self) config = hs.get_config() self.limits_dict = { "upload_size": config.max_upload_size, }

krombel · 2018-05-08T16:22:22Z

synapse/rest/media/v1/limits_resource.py

+        Resource.__init__(self)
+        config = hs.get_config()
+        self.limits_dict = {
+            "upload_size": config.max_upload_size,


You can shorten it by s/config/hs.get_config()/ as you do not store config somewhere.
But that I think that is personal preference...

Eh, the reason for this was to leave it open for more configuration options in the future. Personally I'd be fine with your suggestion if I was only planning to use it once.

But uh, I could go either way with this.

turt2live · 2018-06-20T16:34:52Z

synapse/rest/media/v1/config_resource.py

+        Resource.__init__(self)
+        config = hs.get_config()
+        self.limits_dict = {
+            "upload_size": config.max_upload_size,


m.upload.size

turt2live · 2018-06-20T16:35:42Z

synapse/rest/media/v1/media_repository.py

@@ -756,3 +756,4 @@ def __init__(self, hs):
            self.putChild("preview_url", PreviewUrlResource(
                hs, media_repo, media_repo.media_storage,
            ))
+        self.putChild("limits", MediaLimitsResource(hs))


this is config now

Half-Shot · 2018-07-06T13:28:08Z

Is this a smidge more merge-able now matrix-org/matrix-spec-proposals#1189 is a thing?

concerns addressed

erikjohnston · 2018-07-18T12:11:39Z

synapse/rest/media/v1/config_resource.py

+
+    def render_GET(self, request):
+        respond_with_json(request, 200, self.limits_dict, send_cors=True)
+        return NOT_DONE_YET


Presumably we shouldn't be returning NOT_DONE_YET if we have actually finished the request.

@hawkowl how is this meant to work in twisted?

I'm pretty sure this is correct. Your options for returning from a render_* method are:

return a bytes, which twisted will then render as the body and call request.finish()

return NOT_DONE_YET, which skips the above two.

the implication is that if you have rendered the body and called finish() yourself, you should call NOT_DONE_YET.

Note that this is exactly what happens with a whole bunch of our handlers that look like they are asynchronous, but actually complete synchronously.

HOWEVER. There is a problem here, in that wrap_json_request_handler does important things (1: adds exception handling, though that may be moot here; 2: adds logging). So, can haz wrap_json_request_handler please?

erikjohnston · 2018-07-18T12:12:20Z

Other than weirdness about NOT_DONE_YET, which hopefully hawkowl can give a pointer on, its fine by me.

richvdh · 2018-07-27T08:03:03Z

synapse/rest/media/v1/config_resource.py

+            "m.upload.size": config.max_upload_size,
+        }
+
+    @wrap_json_request_handler


hum; does this actually work?

the docstring on wrap_json_request_handler says:

The handler method must have a signature of "handle_foo(self, request)",
where "self" must have a "clock" attribute (and "request" must be a
SynapseRequest).

I don't think the first is true. You might need a self.clock = hs.get_clock() in the constructor.

richvdh · 2018-07-27T08:13:24Z

retest this please

Half-Shot · 2018-07-27T10:25:04Z

Looks like a 500 on the endpoint. I'll fix.

richvdh · 2018-07-27T11:09:01Z

changelog.d/3184.feature

@@ -0,0 +1 @@
+Add /_media/r0/config


I guess this won't mean much to the average reader of the synapse changelog. You probably need to say it's a new endpoint for clients. A link to the proposal might help.

richvdh · 2018-07-27T11:11:32Z

synapse/rest/media/v1/config_resource.py

+
+    @wrap_json_request_handler
+    def _async_render_GET(self, request):
+        return respond_with_json(request, 200, self.limits_dict)


needs auth?

richvdh · 2018-08-15T18:40:01Z

@matrixbot retest this please

Half-Shot · 2018-08-16T00:27:18Z

@matrixbot retest this please

Half-Shot · 2018-08-16T00:30:46Z

Tests are now passing for this endpoint so I don't know why the other tests are upset.

richvdh · 2018-08-16T12:27:46Z

@matrixbot retest this please

richvdh · 2018-08-16T12:28:00Z

(they were failing due to outdated sytest)

richvdh · 2018-08-16T12:52:43Z

retest this please

SRSLY

Half-Shot · 2018-08-16T13:24:09Z

Thanks!

@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))

Add GET media/v1/limits

53385e8

This was referenced May 3, 2018

Add getMediaLimits to client matrix-org/matrix-js-sdk#644

Merged

GET media/config matrix-org/matrix-spec-proposals#1189

Merged

size > upload_size as per spec feedback

3976352

Half-Shot force-pushed the hs/upload-limits branch from 4a70fcb to 3976352 Compare May 3, 2018 16:30

erikjohnston reviewed May 4, 2018

View reviewed changes

Half-Shot added 2 commits May 8, 2018 15:30

Make MediaLimitsResource Leafy

f8b06fc

Restructure __init__

fee0450

krombel reviewed May 8, 2018

View reviewed changes

Half-Shot added 2 commits June 20, 2018 17:25

Update and rename limits_resource.py to config_resource.py

c06b1cb

media limits => config

b1d8777

turt2live suggested changes Jun 20, 2018

View reviewed changes

turt2live previously requested changes Jun 20, 2018

View reviewed changes

Half-Shot added 2 commits June 21, 2018 11:05

upload_size => m.upload.size

1d53129

/limits => /config

0aa14e4

Half-Shot changed the title ~~Add GET media/v1/limits~~ Add GET media/v1/config Jun 22, 2018

Half-Shot mentioned this pull request Jun 23, 2018

Add tests for /_matrix/media/r0/config matrix-org/sytest#456

Closed

Half-Shot assigned erikjohnston Jun 23, 2018

Half-Shot requested a review from a team July 9, 2018 09:12

Half-Shot added 3 commits July 9, 2018 13:31

Merge branch 'develop' into hs/upload-limits

a7f4ebb

Add import for MediaConfigResource

87fe4f2

Create 3184.feature

48251dc

erikjohnston reviewed Jul 18, 2018

View reviewed changes

erikjohnston assigned hawkowl and unassigned erikjohnston Jul 18, 2018

richvdh assigned Half-Shot and unassigned hawkowl Jul 18, 2018

wrapped

3f520b2

richvdh mentioned this pull request Jul 27, 2018

Test hs/upload limits matrix-org/sytest#466

Merged

richvdh reviewed Jul 27, 2018

View reviewed changes

Half-Shot added 2 commits July 27, 2018 12:01

wrap_json_request_handlering

8cc46ed

linting

bf13c18

richvdh reviewed Jul 27, 2018

View reviewed changes

Half-Shot added 3 commits July 27, 2018 12:17

Auth media/r0/config

bf30fd8

Merge remote-tracking branch 'upstream/develop' into hs/upload-limits

46afacc

isorted this mess

0f6e583

richvdh requested a review from a team August 15, 2018 18:40

Merge remote-tracking branch 'upstream/develop' into hs/upload-limits

b3e005d

richvdh merged commit c151b32 into matrix-org:develop Aug 16, 2018

richvdh mentioned this pull request Oct 15, 2018

Add info about the max upload limit when upload fails #4037

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GET media/v1/config #3184

Add GET media/v1/config #3184

Half-Shot commented May 3, 2018 •

edited by richvdh

Loading

matrixbot commented May 3, 2018

matrixbot commented May 3, 2018

erikjohnston May 4, 2018

Half-Shot May 4, 2018

erikjohnston May 4, 2018

Half-Shot May 4, 2018

erikjohnston May 4, 2018

krombel May 8, 2018 •

edited

Loading

Half-Shot May 8, 2018

turt2live Jun 20, 2018

turt2live Jun 20, 2018

Half-Shot commented Jul 6, 2018

erikjohnston Jul 18, 2018

richvdh Jul 18, 2018

erikjohnston commented Jul 18, 2018

richvdh Jul 27, 2018

richvdh commented Jul 27, 2018

Half-Shot commented Jul 27, 2018

richvdh Jul 27, 2018

richvdh Jul 27, 2018

richvdh commented Aug 15, 2018

Half-Shot commented Aug 16, 2018

Half-Shot commented Aug 16, 2018

richvdh commented Aug 16, 2018

richvdh commented Aug 16, 2018

richvdh commented Aug 16, 2018

Half-Shot commented Aug 16, 2018

Add GET media/v1/config #3184

Add GET media/v1/config #3184

Conversation

Half-Shot commented May 3, 2018 • edited by richvdh Loading

matrixbot commented May 3, 2018

matrixbot commented May 3, 2018

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

krombel May 8, 2018 • edited Loading

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

Half-Shot commented Jul 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikjohnston commented Jul 18, 2018

Choose a reason for hiding this comment

richvdh commented Jul 27, 2018

Half-Shot commented Jul 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh commented Aug 15, 2018

Half-Shot commented Aug 16, 2018

Half-Shot commented Aug 16, 2018

richvdh commented Aug 16, 2018

richvdh commented Aug 16, 2018

richvdh commented Aug 16, 2018

Half-Shot commented Aug 16, 2018

Half-Shot commented May 3, 2018 •

edited by richvdh

Loading

krombel May 8, 2018 •

edited

Loading