Skip to content
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

Clean up user and alias querying for application services #1537

Merged
merged 7 commits into from
Aug 21, 2018

Conversation

turt2live
Copy link
Member

Rendered: see 'docs' status check.

It may be best to review these changes one by one to see the progression.


Split the query user and room APIs out to their own files

Move query APIs to the right heading
Fixes #1325

Take out the false third party network endpoints
Fixes #800

Minor text changes to the query APIs; Keep OpenMarket copyright
The bulk of these APIs were copied from OpenMarket's work - we should preserve the copyright header.

@turt2live turt2live requested a review from a team August 17, 2018 21:46
@turt2live turt2live added this to In review in August 2018 r0 via automation Aug 17, 2018
@Half-Shot Half-Shot mentioned this pull request Aug 17, 2018
11 tasks
@@ -39,7 +40,7 @@ paths:
- in: path
name: roomAlias
type: string
description: The room alias being queried.
description: The URL encoded room alias being queried.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erm, of course it's url-encoded - it's in a URL. we don't say this anywhere else, so specifying it here is confusing imho

{{query_user_as_http_api}}

{{query_room_as_http_api}}


HTTP APIs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a bit more tidying up. In particular, it's odd that the "HTTP APIs" section - which explains that these are APIs which are hit by the homeserver and that the AS must provide them - comes after the "querying" section which of course is a subset of the above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I failed to mention the plan here: All the APIs are being moved out of this section to more relevant areas, making the section blink out of existence. This is the same approach that was used for the server spec to get rid of a similarly named section.

@turt2live turt2live requested a review from richvdh August 20, 2018 18:37
@turt2live turt2live assigned richvdh and unassigned turt2live Aug 20, 2018

Application services wishing to use ``/sync`` or ``/events`` from the Client-Server
API MUST do so with a virtual user (provide a ``user_id`` via the query string). It
is expectected that the application service use the transactions pushed to it to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this has come from master, but I've just noticed it says "expectected" :/

August 2018 r0 automation moved this from In review to Reviewer approved Aug 21, 2018
@turt2live turt2live merged commit 6172d59 into matrix-org:master Aug 21, 2018
August 2018 r0 automation moved this from Reviewer approved to Done (this list will be incomplete) Aug 21, 2018
@turt2live turt2live deleted the travis/as/user-alias-query branch August 21, 2018 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
August 2018 r0
  
Done (this list will be incomplete)
Development

Successfully merging this pull request may close these issues.

None yet

2 participants