Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Create Swagger docs for public API endpoints #340

Merged
merged 2 commits into from
Mar 21, 2019
Merged

Conversation

kellyi
Copy link
Contributor

@kellyi kellyi commented Mar 19, 2019

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

Screen Shot 2019-03-19 at 5 54 59 PM

Testing

  • get this branch then ./scripts/update
  • ./scripts/manage resetdb and ./scripts/manage processfixtures
  • visit localhost:6543 and click on the API link in the navbar, verifying you are taken to the Swagger docs
  • try out the interactive docs for facilities, countries, contributors, and contributor_types
  • back on 6543, create an API token, then copy it to the clipboard
  • return to the API docs and click the "Authorize" button, then enter Token <YOUR_TOKEN> as the auth and click authorize
  • verify that the "/api/facility-lists` section is there

@kellyi kellyi changed the title WIP Create Swagger docs for public API endpoints Create Swagger docs for public API endpoints Mar 20, 2019
@kellyi kellyi requested a review from jwalgran March 20, 2019 20:48
@jwalgran
Copy link
Contributor

Do we know what this [ BASE URL: ] thing is?

Screen Shot 2019-03-20 at 3 45 41 PM

@kellyi
Copy link
Contributor Author

kellyi commented Mar 20, 2019

Interesting, we should be able to add that, however:

marcgibbons/django-rest-swagger#681

'licenseUrl': 'https://github.com/open-apparel-registry/open-apparel-registry/blob/develop/LICENSE', # noqa
'title': 'Open Apparel Registry API',
},
'USE_SESSION_AUTH': False,
Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@kellyi
Copy link
Contributor Author

kellyi commented Mar 21, 2019

I tried out adding a url argument to get_swagger_view and also adding BASE_PATH and BASE_URL values to SWAGGER_SETTINGS. None of these worked. I'll post on the issue linked above to ask if anyone has discovered a workaround.

Copy link
Contributor

@jwalgran jwalgran left a 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
Copy link
Contributor

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.

Copy link
Contributor Author

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

"file_name": "11.csv",
"is_active": true,
"is_public": true,
"items": [
Copy link
Contributor

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.

Screen Shot 2019-03-20 at 4 14 53 PM

Copy link
Contributor Author

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

Screen Shot 2019-03-21 at 11 36 29 AM
Screen Shot 2019-03-21 at 11 36 37 AM

Note that the parameters are still not accurate for the facility lists endpoints.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

19e788f

shortens the "next" url to make it possible to remove # noqa

Copy link
Contributor

@jwalgran jwalgran left a 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.

@jwalgran jwalgran assigned kellyi and unassigned jwalgran Mar 21, 2019
Kelly Innes added 2 commits March 21, 2019 13:56
- 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
@kellyi
Copy link
Contributor Author

kellyi commented Mar 21, 2019

Thanks! #349 is for fixing the facilities list documentation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants