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

Stop using drf-spectacular #542

Merged
merged 2 commits into from
Sep 8, 2021
Merged

Stop using drf-spectacular #542

merged 2 commits into from
Sep 8, 2021

Conversation

sbs2001
Copy link
Collaborator

@sbs2001 sbs2001 commented Aug 25, 2021

  • Use Redoc instead of swagger
  • Don't rely on CDN
  • Fix docs for bulk_search API

Fixes #454 , #446

Signed-off-by: Shivam Sandbhor [email protected]

- Use Redoc instead of swagger
- Don't rely on CDN
- Fix docs for bulk_search API

Signed-off-by: Shivam Sandbhor <[email protected]>
@pombredanne
Copy link
Member

Just curious: what's the rationale for selecting redoc?

@pombredanne
Copy link
Member

Redoc comes from :

To get the code in a script, first get the JSON api at https://registry.npmjs.org/redoc
Then under versions/dist get the dist tarball at https://registry.npmjs.org/redoc/-/redoc-2.0.0-rc.56.tgz

This is overall a messy provenance and therefore license.

@pombredanne
Copy link
Member

This could be a better source than the npm package https://cdn.jsdelivr.net/npm/redoc@next/bundles/redoc.standalone.js and this is also messy license.

@Hritik14
Copy link
Collaborator

Hritik14 commented Sep 2, 2021

Steps to take about licensing after merging this PR: #549

@sbs2001 sbs2001 merged commit 64d52ec into main Sep 8, 2021
@pombredanne pombredanne deleted the use_redoc branch September 28, 2021 06:42
@tfranzel
Copy link

tfranzel commented Oct 1, 2021

Hi,

I watched this a bit from the sidelines and I have to admit I'm a bit puzzled. Maybe I have been too imprecise in my statements.

nobody is forcing you to use the CDN. if you are so inclined, you are free to change those settings and host your swagger-ui build yourself. that is exactly the reason why the setting is there.

You then partially did what I recommended; ingesting the distribution builds into your static folder. So far so good. However, instead of simply setting 'REDOC_DIST': '//cdn.jsdelivr.net/npm/redoc@next' to 'REDOC_DIST': 'YOUR_STATIC_ROOT/api_doc', you went ahead removed spectacular altogether and hard-coded the schema. You could call that throwing out the baby with the bath water.

The whole point of spectacular is not having stale, outdated hard-coded API specifications. This kind of defeats the purpose of the whole thing but I digress. But on a lighter note, we just added your request: https://github.com/tfranzel/drf-spectacular#self-contained-ui-installation

This installs https://github.com/tfranzel/drf-spectacular-sidecar, which is an optional package containing all those assets. The package sources from jsdelivr and releases a PyPi package containing those static files. Basically what you did in this PR. Just some food for thought. Have a good day!

@pombredanne
Copy link
Member

@tfranzel FYI, I am working to reinstate drf-spectacular with swagger which is much better than a plain redoc and this before we are effectively going public :)
Thank you ++ for the sidecar publication!
You rock

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.

drf-spectacular is fetching JS from CDN
4 participants