Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MSC2695: Get event by ID over federation #2695
base: old_master
Are you sure you want to change the base?
MSC2695: Get event by ID over federation #2695
Changes from all commits
5f9c438
5fe216f
596a28e
2a266ec
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small quibble but this feels inconsistent with the other APIs, as a) it has nothing to do with
/events
and b) the rooms API uses.../event/...
singular. I'd vote we use/event/{event_id}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'd be happy with that as well. Only went for the current path since it had already existed before, but your suggestion seems fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like it could leak sensitive data. Why is this needed, rather than just 404-ing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One main use case for this part is to allow the new version of
matrix.to
URIs which might specify only an event ID to still offer a description of the room that event is in via thematrix.to
interstitial site.matrix.to
would use this API to at least find out room ID, and it can then use the room directory API to learn more about the room if it's not world-readable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should only return the room ID if the room "public", i.e. that the server is happy to leak metadata about it? That way we only return the room ID if fetching metadata would return something useful? (Or perhaps merge the two APIs so that you can attempt to fetch room metadata with just the event ID?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a bit of an awkward API shape to represent, but can probably be done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open to suggestions on this; the main goal was to at least return the room ID. I structured the result by just removing the other fields from the "full" response above, but I am sure other patterns would work equally well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an idea would be to let the deprecated endpoint die and introduce a whole new endpoint which does something like:
where
event
is the full thing and optional under the conditions outlined already.