Be consistent about endpoint names

[richvdh]

Sometimes, we called this keys query, sometimes /keys/query, and sometimes, just for variety, keys/query. Generally in Matrix we talk about /keys/query so let's standardise on that.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7e53c68) 82.37% compared to head (766bb65) 82.37%.
Report is 21 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2857   +/-   ##
=======================================
  Coverage   82.37%   82.37%           
=======================================
  Files         211      211           
  Lines       21604    21604           
=======================================
  Hits        17796    17796           
  Misses       3808     3808           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bnjbvr]

I don't want to start a bikeshedding discussion, but… I find that adding the quotes and slashes makes the whole thing unwieldy, and more cumbersome than it ought to be. Plus, we're not doing this for every single request endpoint, so we're breaking consistency in other ways if we started doing it like that here. Also, I find it more human to talk about a "keys query response" rather than a "/keys/query response"; the former is even more in line with our import renames e.g. KeysQueryResponse. And, I'm 100% certain I'll forget about this new convention, since it's very not natural to me 🙃

Out of curiosity: apart from consistency with other projects, were there objective reasons to prefer this writing?

[richvdh]

I don't want to start a bikeshedding discussion, but… I find that adding the quotes and slashes makes the whole thing unwieldy, and more cumbersome than it ought to be. Plus, we're not doing this for every single request endpoint, so we're breaking consistency in other ways if we started doing it like that here. Also, I find it more human to talk about a "keys query response" rather than a "/keys/query response"; the former is even more in line with our import renames e.g. KeysQueryResponse. And, I'm 100% certain I'll forget about this new convention, since it's very not natural to me 🙃

Out of curiosity: apart from consistency with other projects, were there objective reasons to prefer this writing?

That's a compelling reason in itself tbh, especially when this SDK is just being used for a small part of a larger application :). And of course while you'll 100% forget about a convention of "/keys/query", I'll 100% forget about a convention of "keys query" since I'm used to using the former style in spec, Synapse, and js-sdk. I think we'll have to resign ourselves to accepting that whatever convention we adopt, it's going to feel alien to some of us.

IMHO the main reason to prefer "/keys/query" over "keys query" is that it very clearly demonstrates that we are talking about a particular REST endpoint, as opposed to (say) a storage query, or even another rest endpoint which happens to deal with similarly named concepts. While it's a bit of a stretch to imagine the Rust SDK ever calling /key/v2/query, it certainly gets involved in "key uploads" which are nothing to do with the /keys/upload endpoint (I'm thinking of backups, for example).

All that said: I don't care that much as long as we can agree that having log lines which switch randomly between /keys/query, keys/query and keys query is terrible.

[bnjbvr]

I'm against this change, but others in the Rust team (since we'll be the ones mostly interacting with all that code) don't feel as strongly against it, so I'll let them review.

[richvdh]

For context, there was some discussion of this in an internal room. It felt like most other people find the "/keys/query" form helpful to reflect the fact we're talking about an actual endpoint rather than, more generally, about the "keys querying process".

Still, I'm happy to find a compromise here. Some thoughts:

• I strongly believe that, whatever we do, we should at least be consistent in what we write in log lines. This is not the case today, even among references to the same endpoint.
• I'd like us to be consistent in other parts of the code too (eg comments, documentation, expect calls), but don't care as much about it.
• I realise that this doesn't bring the whole codebase into line; it's just addressing a small part of it which I found particularly egregious (in particular: there was no one thing I could search for in my logs to find out what was going on with requests to /keys/query). It's just a papercut but honestly I spent most of last week cutting my hands to shreds trying to understand what was causing Element-R: sending messages takes several seconds element-hq/element-web#26375.
• If it's the backticks that are causing consternation, I will happily rip them out.