search

data-for-change / anyway

ANYWAY - Car accidents map
http://www.anyway.co.il
MIT License
77 stars 242 forks source link

Add swagger for API documentation #1780

Open atalyaalon opened 3 years ago

atalyaalon commented 3 years ago

I suggest using Flask-RESTX's

atalyaalon commented 3 years ago

@yuvalbl FYI - please let us know if you have any specific requests

yuvalbl commented 3 years ago

Seems like you'll need to rewrite you entire API. is that OK with you?

atalyaalon commented 3 years ago

I'm not sure it means rewriting the whole API @ziv17 can you evaluate the work needed here? For start we should document only the 2 APIs our FE team is using - both news flash API and infographics API - the documentation of the rest of the APIs used by our map FE are not important for now.

ziv17 commented 3 years ago

Yes, I will check cost of documenting a small item to assess.

ziv17 commented 3 years ago

Hi @atalyaalon, @yuvalbl, I did a small check. It seems that just adding Swagger documentation does not require meaningful change in our code. Actually processing the input parameters with REST-X constracts (so you cannot reach a situation were the code behaves differently from the documentation) requires some change, but not large. Having also the marshaling of the response generated via REST-X constructs may be a bigger change.

atalyaalon commented 3 years ago

I suggest starting with the news flash API - start with the low hanging fruits - documentation, input parameters, and evaluate the cost of marshaling the response - then we'll decide if that part is worth the effort.

atalyaalon commented 3 years ago

@ziv17 swagger looks very good! The FE team, together with @yuvalbl are very excited towards it Can we make sure we have only 2 APIs in swagger for now?

  1. infographics-data - can we add the lang var?
  2. news-flash (Perhaps after @afedj will add the urban accidents - there might be a change in vars there)
  3. remove ​/api​/news-flash-new and ​/api​/news-flash​/{news_flash_id}