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

API: swagger doc and definition for GET buildings #1308

Open
2 tasks
tomalrussell opened this issue Aug 6, 2019 · 9 comments
Open
2 tasks

API: swagger doc and definition for GET buildings #1308

tomalrussell opened this issue Aug 6, 2019 · 9 comments
Assignees

Comments

@tomalrussell
Copy link
Contributor

Initial setup and documentation for the core buildings API call:

Resources

@mz8i do you have a link for the typescript-swagger tool you mentioned earlier?

@mz8i
Copy link
Contributor

mz8i commented Aug 6, 2019

@tomalrussell the libraries are https://www.npmjs.com/package/typescript-rest and https://www.npmjs.com/package/typescript-rest-swagger - they aren't hugely popular right now so might be something to consider. I have used them in a couple projects

@tomalrussell
Copy link
Contributor Author

Thanks @mz8i - looks like https://www.npmjs.com/package/tsoa is in the space and may be worth evaluating too.

Can you evaluate options and put together a proof of concept?

By default, I'd be cautious about adding dependencies and might prefer just using the swagger codegen to stub out methods and generate static HTML UI. I'm happy to look at what's available, though, and keen to see some running code that we can test out,

The key goals as far as I see it are:

  1. Consistent API for core data operations (to be used by web app, any future apps, programmatic upload/download tools)
  2. Neat swagger documentation site for technical users
  3. Refactor for maintainability

@mz8i
Copy link
Contributor

mz8i commented Aug 7, 2019

tsoa looks good too. I don't have experience with it but it seems to be slightly more popular.

So far the differences I can see are:

  • tsoa contains swagger generation functionality, while typescript-rest needs to be used together with typescript-rest-swagger
  • typescript-rest sets up the routes at run-time whereas tsoa requires running a code generation step for generating the route files
  • typescript-rest is tied to Express whereas tsoa, due to its code generation approach, can target different middlewares like Express, Koa etc

It is worth noting that using any of the above relies on #348 (switching to TypeScript) - so that needs discussing and potentially implementing first.

The alternatives would be:

  • writing the swagger spec file manually for the existing code
  • using some other approach like https://www.npmjs.com/package/swagger-jsdoc to embed the swagger spec in documentation comments in code - the benefit is having the spec and the code in the same place, but the downside is that you still need to write the spec manually so there is space for errors/inconsistencies

@tomalrussell
Copy link
Contributor Author

I do like the sound of swagger-jsdoc: seems pretty pragmatic, relatively quick win option.

I think the API should never grow too big, so not too too worried about keeping it consistent - the main thing will be keeping on top of building attributes as we fill out the remaining sections.

So, suggest:

[1] http://www.catb.org/~esr/jargon/html/Y/yak-shaving.html

@mz8i
Copy link
Contributor

mz8i commented Aug 8, 2019

I started out with moving out the buildings API code into a routes file and a controller file to decrease the size of server.js a bit: mz8i@330642c

I also manually created an initial openapi.yml (Swagger is now called OpenAPI) file with the specification of some of the endpoints: mz8i@bd1645c

Some thoughts:

  • I did it in a standard openapi.yml file for now, as I figured out splitting the spec into jsdoc comments doesn't add much value in terms of keeping the documentation consistent with code, while the tooling available is much better for editing a single OpenAPI/Swagger file. If we want, I can later split it and put into comments in source files.
  • some endpoints like POST /building/:building_id.json rely on user_id in user session. Swagger has some options for describing authorization but I am not sure if this particular method will be possible to describe in the spec.

Any comments welcome, otherwise I will continue describing the other endpoints and possibly refactoring.

@polly64
Copy link
Contributor

polly64 commented Jan 9, 2024

@matkoniecz can you close?

@matkoniecz
Copy link
Contributor

Depends on whether we want to have documentation of API and plan to spend time on that.

Good documentation would be useful for more automated edits.

@polly64
Copy link
Contributor

polly64 commented Jan 10, 2024

@matkoniecz shall I move to core then for discussion with international group?

@matkoniecz
Copy link
Contributor

yes

@polly64 polly64 transferred this issue from colouring-cities/colouring-britain Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants