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

riot uses unstable APIs #5325

Open
richvdh opened this Issue Oct 18, 2017 · 11 comments

Comments

Projects
None yet
5 participants
@richvdh
Copy link
Member

richvdh commented Oct 18, 2017

There are a whole bunch of places in the js-sdk which call apis under the /unstable/ prefix; these are in turn used by riot-web. The ENTIRE POINT of unstable is to denote APIs which you can't rely on because they might change.

So WTF are we doing using them in released versions of riot?

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Oct 18, 2017

This should be considered a release-blocker imho

@krombel

This comment has been minimized.

Copy link
Contributor

krombel commented Oct 26, 2017

Currently there are some endpoints that are not exposed by synapse on /r0/ but on /unstable/ only.
This issue is blocked on that missing ability.

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Oct 26, 2017

yup. could you clarify what they are?

@dbkr

This comment has been minimized.

Copy link
Member

dbkr commented Oct 30, 2017

Here's a list of things known to be using the /unstable prefix in the js-sdk:

  • /login/cas/redirect (getCasLoginUrl)
  • /account/deactivate
  • /account/3pid/delete
  • /devices
  • /devices/$device_id (PUT and DELETE)
  • /keys/upload/$deviceId
  • /keys/query
  • /keys/claim
  • /keys/changes
  • /sendToDevice/$eventType/$txnId
  • /thirdparty/protocols
  • /thirdparty/location/$protocol

It looks like APIs that are in synapse unstable but not r0 are:

  • /account/3pid/delete
  • /login/saml2
  • /login/cas/redirect
  • /login/cas/ticket
  • /createUser

In other words, I think the answer to the question above is the CAS endpoints and 3pid deletion.

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Nov 6, 2017

In other words, I think the answer to the question above is the CAS endpoints and 3pid deletion.

This is only true as of matrix-org/synapse#2579 (which has not yet been released). Before that I think that all of the things in the top list (except possibly /account/deactivate ?) were unstable-only.

@lampholder lampholder added this to the 0.15 - candidates milestone Mar 13, 2018

@lampholder

This comment has been minimized.

Copy link
Member

lampholder commented Mar 13, 2018

If supporting old versions of riot is something we've committed to do, then releasing versions of riot dependent on unstable APIs effectively commits us to supporting those unstable APIs in their current shape (rendering them stable by accident and not by choice).

Which is obviously terrible.

But, if we have already released those versions of Riot, then I don't see the particular need to block 0.14 on this issue. Certainly it's a high priority techincal debt maintenance item that needs to be resolved, and certainly each released version of Riot with these dependencies only makes the number of human users dependent on those unstable APIs greater which is undoubtably a BadThing.

But since this problem wouldn't be resolved by simply fixing our /unstable dependencies in 0.14, I'm going to bias towards this not being a release-blocker for 0.14. I will moot it for consideration in 0.15, though.

@richvdh I hope that makes sense - if you want to fist fight me on this we can schedule some time tomorrow 😛

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Mar 13, 2018

fine, but continuing to punt it will result in toy-throwing tantrums.

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Mar 13, 2018

It also uses /media/v1 for some of the media apis, which is also incorrect.

@lampholder lampholder modified the milestones: 0.15 - candidates, 0.16 - candidates Mar 26, 2018

@lampholder lampholder modified the milestones: RW008 - Candidates, RW009 - Candidates Apr 25, 2018

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented May 14, 2018

Be warned that my toys are nearing the edge of the pram again.

I think I might drop support for the unstable prefix in the next version of synapse. These APIs are unstable, so I can do that, right?

@ara4n

This comment has been minimized.

Copy link
Member

ara4n commented Feb 8, 2019

@lampholder can we get this on the radar for after riot 1.0 please?

@lampholder

This comment has been minimized.

Copy link
Member

lampholder commented Feb 8, 2019

/me has made a note

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