-
Notifications
You must be signed in to change notification settings - Fork 37
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
API: Find / Update Pairing #3
Comments
I've started working on these endpoints, with documentation on the wiki. |
The current API feels quite "RPCish" and might not be easily extensible. E.g., what happens if we switch black/white and request: The standard REST semantics would make the API look something like this:
Updating works by using PUT on the ID: Creation of new pairings: Deletion of pairings: What do you think? |
We had some discussion about this before (whether to use a RESTful API versus API methods specifically targeted to Chesster's needs) and decided to keep the implementation as simple as possible since we have very limited uses for the API. That said, you're right that these two endpoints line up very well with REST. I'll leave it up to @lakinwecker since he's the one that's going to be writing the code to consume the API. |
Thanks for the feedback! You are correct that these are RPC calls. I believe this is exactly what we need. I would also call the suggested URLs and concepts RPC as well. See below for a bit of a side-track into the depths of what makes REST amazing, and a blog post by Roy Fielding on why most people only need RPC and why most APIs that people build these days are actually RPC and why that's OK. For v1 - I'm not keen on spending time building a generalized API let alone a fully RESTful one. We have limited resources, and those resources would be better spent on making the lives of the moderators easier. A handful of RPC calls are exactly what we need given our resources. That being said, I do prefer the URL formats you suggestd. Having parameters in the query-string if possible, it's easier to see that the name relates to the value than having them in to the path. But whether that's: Also, we have no need to delete a pairing from the API. Nor do we need to to create a new pairing from the API. Hence my statement about not needing a generalized API. Small rant
http://roy.gbiv.com/untangled/2008/rest-apis-must-be-hypertext-driven Unless we go way out of our way to satisfy these constraints along with the myriad of other constraints that REST requires, then we are building RPC endpoints. That's ok though, most people only need RPC. The full benefits of REST aren't necessary. The primary reason to aim for the full benefits of REST is to make a protocol that's designed to outlive organizations and specific software (think decades and the myriad of browsers and servers that have come and gone). We're making a set of RPC endpoints to do a few specific things and if they run the league for 2 years, that will be an outstanding success. I highly recommend that blog post, it's a good read. |
@lakinwecker You have a valid point with many APIs being labeled as REST while not at all complying to the semantics. The discussion on REST is long, so let me just give my 2¢. I'm not supposing to create a full blown HATEOAS REST API for some simple internal call. That blog post you linked talks about APIs used to connect systems and that is where HATEOAS shines. I suggest Richardsons Maturity Model as it allows to reason about APIs that are following the REST semantics while not being true REST APIs. Personally, I use REST semantics for even the simplest kind of my internal APIs. The reason mainly being the consistent semantics and the ease of use with external libraries. Adding a query parameter does not break my routes, neither does changing their order. I never actually used Django but make sure to check out Flask to see how beautiful and easy APIs are defined there. All the query calls that I described above would boil down to a single route and about 5 lines of code. Hook up an SQLite DB + auth and you have your "Pairing Service" ready to ship. |
@cyanfish Couldn't there be more than one pairing for any of the
|
@chris There could theoretically be more than one, but it's not a normal use case and AFAIK the client has no meaningful way to handle multiple results. Instead I have the error case "ambiguous". |
Agreed. My client doesn't need lists for this RPC method. |
@chris: Regarding REST and HATEOAS - you make good points. I haven't looked at what flask does, but I know that django has equivalent REST style libraries. As for the external 3rd party libraries - we don't have a need for those at the moment. Django's auto-admin will suffice and give us plenty of flexibility with the first set of features. Again, neither cyanfish (I think?) nor I (I'm sure) have the time to look into it and what cyanfish has already done is good enough. If you're offering to contribute said API and then maintain it for us, I'd be happy to review a PR with that in it. Otherwise, making or supporting it is beyond what I'm going to have time to do while we focus on getting an initial version working. |
I agree that the API is perfectly fine for the use case, if you ever need to extend beyond what is already built than I'd happily volunteer to help. |
Closing this since these endpoints are fully functional. |
Make an API method for searching for a pairing based on one or two player names, with an optional league/season filter. It should return the complete set of information for each pairing.
Make an API method for updating the gamelink, or result of a pairing.
Currently chesster does the "Is moderator or is a member of the pairing" check so we could effectively allow this to work for any authenticated user.
The text was updated successfully, but these errors were encountered: