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

patch: More verbose yaml errors #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

verpeteren
Copy link

When the docstring is invalid yaml/swagger, the generated swagged documentation
just shows:

Invalid Swagger
⚠ Swagger document could not be loaded from docstring ⚠

This change makes the docstring parsing step a bit more verbose to indicate that
there are problems. It is still not ideal but it is an improvement over guessing/
copy-pasta into a validator.

How to test that this is working:

  1. create a aiohttp route with docstring, add it to the app with the swagger plugin
  2. start the app
  3. navigate to the /api/doc page and find the route
    observe that the route is displayed correctly in the swagger page
  4. change the docstring to invalid yaml (e.g. by indenting with tabs)
  5. (re) start the app
    observe that there is more verbose output
  6. (re) navigat ot the /api/doc page and find the route
    observe that the route now probably is labeled as 'Invalid Swagger'

When the docstring is invalid yaml/swagger, the generated swagged documentation
just shows:

```
Invalid Swagger
⚠ Swagger document could not be loaded from docstring ⚠
```

This change makes the docstring parsing step a bit more verbose to indicate that
there are problems. It is still not ideal but it is an improvement over guessing/
copy-pasta into a validator.

How to test that this is working:
1) create a aiohttp route with docstring, add it to the app with the swagger plugin
2) start the app
3) navigate to the /api/doc page and find the route
   observe that the route is displayed correctly in the swagger page
4) change the docstring to invalid yaml (e.g. by indenting with tabs)
5) (re) start the app
   observe that there is more verbose output
6) (re) navigat ot the /api/doc page and find the route
   observe that the route now probably is labeled as 'Invalid Swagger'
@codecov-io
Copy link

codecov-io commented Dec 18, 2019

Codecov Report

Merging #84 into master will decrease coverage by 3.43%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #84      +/-   ##
=========================================
- Coverage   94.73%   91.3%   -3.44%     
=========================================
  Files           4       4              
  Lines         133     138       +5     
=========================================
  Hits          126     126              
- Misses          7      12       +5
Impacted Files Coverage Δ
aiohttp_swagger/helpers/builders.py 86.66% <0%> (-5.1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db559ae...a85683e. Read the comment docs.

@goranpavlovic goranpavlovic force-pushed the master branch 2 times, most recently from 10303be to ea32115 Compare July 3, 2020 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants