-
Notifications
You must be signed in to change notification settings - Fork 13
Create Swagger docs for public API endpoints #340
Conversation
Interesting, we should be able to add that, however: |
'licenseUrl': 'https://github.com/open-apparel-registry/open-apparel-registry/blob/develop/LICENSE', # noqa | ||
'title': 'Open Apparel Registry API', | ||
}, | ||
'USE_SESSION_AUTH': False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we may be able to configure the base url somewhere here so I'll try it out tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ended up not working. I asked a question here marcgibbons/django-rest-swagger#681 about whether there's an alternate way to fix this, so if someone answers we can circle back to this later.
I tried out adding a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start. Thats for struggling with We will be evolving this over time, I imagine.
I made a few comments.
@@ -1,5 +1,6 @@ | |||
base32-crockford==0.3.0 | |||
boto3==1.9.92 | |||
coreapi==2.3.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a sentence or two to the commit message explaining why we need to pull in this additional library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I'll update the commit message. Essentially this is the DRF recommended library to use for creating a documentable schema: https://www.django-rest-framework.org/api-guide/schemas/#schemas
src/django/api/views.py
Outdated
"file_name": "11.csv", | ||
"is_active": true, | ||
"is_public": true, | ||
"items": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer correct since we have merged #341. We now return item_count
instead of items. There is new items
endpoint that is being picked up automatically, but the page
and pageSize
parameters aren't appearing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool -- I updated these in 0404a56
Note that the parameters are still not accurate for the facility lists endpoints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shortens the "next"
url to make it possible to remove # noqa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the fixes. Looks good.
Please make sure we have logged a follow up issue that enumerates the remaining items we have to fix.
- add Django REST Swagger dependency - partition urlpatterns into public_apis and internal_apis - configure Django REST Swagger - add doc strings for public API endpoints
- add coreapi library as recommended in https://www.django-rest-framework.org/api-guide/schemas/#schemas - add filter backend and declare schema for /api/facilities query parameters
Thanks! #349 is for fixing the facilities list documentation. |
Overview
This PR creates a set of Swagger docs for the application's public API endpoints, accessible at
/api/docs
.The inline docstrings are currently accurate for all endpoints, and the contributors, contributor types, countries, and facilities endpoints are both documented and working through the UI.
I had substantial trouble getting the facility lists documentation to work, so I delayed fleshing that out until #349. Django REST Swagger deprecated using inline YAML to declare a schema and fields for each endpoint in version 2, and lower versions are not compatible with our existing setup.
Conceivably we could just drop the
api/facilities-list
endpoints from the publicly documented API altogether for now and come up with a better solution later. The other interactive endpoints are working as expected.Connects #88
Demo
Testing
./scripts/update
./scripts/manage resetdb
and./scripts/manage processfixtures
Token <YOUR_TOKEN>
as the auth and click authorize