MSC1902: Split the media repo into s2s and c2s parts #1902
Conversation
turt2live
changed the title
|
I don't think splitting on the version string is a good idea. It by far the easiest, sure, but it doesn't feel good semantically |
|
What happens when, for unknown future reasons, the version(s) need to change? It seems to me that this point should be at least addressed in the proposal. |
|
@abeluck that's already handled by nature of the APIs being versioned @jcgruenhage suggestions welcome - I'm not particularly comfortable with moving them, but that doesn't mean its off the table As an aside: please leave comments on the diff so this is threaded. |
|
I've altered this proposal slightly to diminish the importance of authentication from the client-server side, leaving that for another MSC to handle. Aside from that, the MSC is largely the same and I think has been sitting here for long enough to move onto the next stage of idle activity: @mscbot fcp merge |
|
This FCP proposal has been cancelled by #1902 (comment). Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people: Concerns:
Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for information about what commands tagged team members can give me. |
mscbot
added
disposition-merge
proposed-final-comment-period
|
oh, hah, this requires an implementation. Sorry for the noise. @mscbot fcp cancel |
|
This proposal is not in FCP. |
|
@mscbot concern No implementation |
|
@mscbot fcp cancel |
mscbot
added
proposal-in-review
and removed
disposition-merge
proposed-final-comment-period
turt2live
added
the
needs-implementation
| | `/_matrix/media/:version/download/:origin/:mediaId` | `/_matrix/client/r0/media/download/:origin/:mediaId` | `/_matrix/federation/v1/remote_media/download/:origin/:mediaId` | | ||
| | `/_matrix/media/:version/thumbnail/:origin/:mediaId` | `/_matrix/client/r0/media/thumbnail/:origin/:mediaId` | `/_matrix/federation/v1/remote_media/thumbnail/:origin/:mediaId` | | ||
| | `/_matrix/media/:version/upload` | `/_matrix/client/r0/media/upload` | N/A | | ||
| | `/_matrix/media/:version/preview_url` | `/_matrix/client/r0/media/preview_url` | N/A | | ||
| | `/_matrix/media/:version/config` | `/_matrix/client/r0/media/config` | N/A | |
There was a problem hiding this comment.
Since this has been last changed a while ago;
| | `/_matrix/media/:version/download/:origin/:mediaId` | `/_matrix/client/r0/media/download/:origin/:mediaId` | `/_matrix/federation/v1/remote_media/download/:origin/:mediaId` | | |
| | `/_matrix/media/:version/thumbnail/:origin/:mediaId` | `/_matrix/client/r0/media/thumbnail/:origin/:mediaId` | `/_matrix/federation/v1/remote_media/thumbnail/:origin/:mediaId` | | |
| | `/_matrix/media/:version/upload` | `/_matrix/client/r0/media/upload` | N/A | | |
| | `/_matrix/media/:version/preview_url` | `/_matrix/client/r0/media/preview_url` | N/A | | |
| | `/_matrix/media/:version/config` | `/_matrix/client/r0/media/config` | N/A | | |
| | `/_matrix/media/:version/download/:origin/:mediaId` | `/_matrix/client/v3/media/download/:origin/:mediaId` | `/_matrix/federation/v1/remote_media/download/:origin/:mediaId` | | |
| | `/_matrix/media/:version/thumbnail/:origin/:mediaId` | `/_matrix/client/v3/media/thumbnail/:origin/:mediaId` | `/_matrix/federation/v1/remote_media/thumbnail/:origin/:mediaId` | | |
| | `/_matrix/media/:version/upload` | `/_matrix/client/v3/media/upload` | N/A | | |
| | `/_matrix/media/:version/preview_url` | `/_matrix/client/v3/media/preview_url` | N/A | | |
| | `/_matrix/media/:version/config` | `/_matrix/client/v3/media/config` | N/A | |
| Instead of using `/_matrix/media` for both servers and clients, servers must now use | ||
| `/_matrix/federation/v1/remote_media` and clients must use `/_matrix/client/r0/media`. | ||
|
|
||
| To support backwards compatibility and logical usage of the media repo, only downloads |
There was a problem hiding this comment.
I don't really understand how this supports backwards compatibility.
Is the idea that clients and requesting-servers should use the new endpoints and fall back to the old ones on 400? Or do we just have to wait for everyone to support the new endpoints before we can switch to using them?
There was a problem hiding this comment.
TLDR: We should fix this MSC to use fallback semantics.
This paragraph certainly mixes two different ideas in it:
- Federation and client-server should be different things (which is hopefully covered by the rest of the proposal and doesn't make sense to mention here)
- We're saying that the existing
/media/:versionendpoints are deprecated in favour of the new ones, but function as they do today (supporting both federation and client requests). The new ones would validate via authentication, where appropriate, that the correct thing is being called.
Now, is this the correct way to handle backwards compatibility in not-2018 Matrix? I don't think so. We probably just want to deprecate the existing endpoints (so we can remove them later) and rely on fallback semantics for servers who care. Spec-wise, we don't really need to define how the fallback works once the ecosystem has sufficient adoption, but the MSC should definitely cover it.
There was a problem hiding this comment.
It is frustrating that we still don't have a defined behaviour for unrecognised endpoints (MSC3743), making fallback 50 times harder than it should be.
|
I've basically rewritten this as MSC3916. |
|
Closing in favour of #3916 |
turt2live
added
the
obsolete
Rendered
References: