NetBox currently uses drf_yasg to auto-generate an API schema from code and publish the swagger docs explorer. It has become harder to support drf_yasg has it has been the source of a number of recent bugs. It is also clear its vision and DRF have diverged. DRF now natively support schema auto-generation as well several API docs formats (coreapi, swagger, etc). From a maintainability standpoint, it will be much easier for us to support this natively within DRF.
NetBox needs an API docs explorer and a published API schema spec. drf_yasg is hard to maintain and DRF now has similar native functionality.
Additionally, this represents an opportunity to fix many of the API docs errors relating to field types and foreign key constraints.
None
This will remove a dependency on drf_yasg but may introduce a DRF codependency on coreapi, if we choose to go that specific route (remains to be seen).
CoreAPI is on its way out from DRF anyway https://www.django-rest-framework.org/community/3.9-announcement/#whats-next
We're planning to iteratively work towards OpenAPI becoming the standard schema representation. This will mean that the coreapi dependency will gradually become removed, and we'll instead generate the schema directly, rather than building a CoreAPI Document object.
OpenAPI has clearly become the standard for specifying Web APIs, so there's not much value any more in our schema-agnostic document model. Making this change will mean that we'll more easily be able to take advantage of the full set of OpenAPI functionality.
@tyler-8 thank for that! That takes care of that decision. I like OpenAPI anyway.
Seems like we're running into this issue with generating an OpenAPI schema from the nested routers.
Shooting to have this done for v2.6, but we'll see how involved it gets.
As described in the PR that moved netbox over to drf_yasg (#1930), the previous lib was not including response schemas in the OpenAPI definitions. It would be great if the built-in doc support included them. Hopefully it gets the programmatic definitions right as well as the docs. But if not, I understand the importance of accurate docs.
Hello! (sole) maintainer of drf-yasg here. This issue caught my attention, so please don't mind me chiming in 馃槃
netbox is quite an interesting real-world user of my library, so I'd be curious to hear about the difficulties you encountered while integrating drf-yasg and why you believe they could be avoided by using the builtin DRF schemas.
As @davcamer pointed out, the main goal of drf-yasg is to provide a programatically correct OpenAPI schema. (interactive) documentation comes as a secondary benefit of that, albeit a nice one. However, since Python is a very dynamic language, some effort in keeping a correct correspondence between code behaviour and typing metadata is required to achieve this goal. So I'd always love to hear about ways to make this process easier and improve the API/usability of drf-yasg.
That being said I view the current state of DRF docs as far from good enough even for the purpose of docs - response definitions are completely missing and nested request bodies are quite buggy. As you noted above DRF is making efforts to move to OpenAPI schemas, which is not a trivial task and will probably take a while to get done. It also means that DRF docs will likely hit many of the same problems and limitations as drf-yasg.
It is also clear its vision and DRF have diverged.
The vision between the two was always quite different, but still compatible, I'd like to believe 馃槃 . In a perfect world DRF upstream will sometime reach a point where there is no need for 3rd party schema support.
Hi @axnsan12, thank you for joining us! Let me start off by acknowledging that I've done a poor job of managing NetBox API documentation efforts overall. It hasn't really been a priority until recently, and something I aim to do much better with in v2.6 and onwards.
I think there are/were actually several different but related issues driving this. One is simply failure at times to differentiate among the various components involved in rendering the API documentation, which I'll list here for everyone's reference:
Collectively, these are all just "the API docs" as far as NetBox users are concerned, which leads to confusion. The first "real" implementation of a REST API started with NetBox v2.0, and we adopted drf-yasg in NetBox v2.3.2 (#1930). Since then, we've encountered a number of issues concerning invalid introspections and UI limitations, but not necessarily related to any particular component. As an example, introspection is currently broken for many (all?) SerializerMethodFields. This isn't an issue with any particular component, of course; it's merely a limitation inherent to that type of field, which we've yet to address.
As of v3.9.0, Django REST Framework supports OpenAPI natively, which I believe prompted this specific issue (#2665). The prospect of dropping a dependency is always appealing, so I started to explore this option, but as you point out its current capabilities are still fairly limited.
Other issues that have come up deal with the browsable documentation. One example: Swagger generates a single flat list of endpoints under /api/ instead of nesting them under their respective apps, and rendering all of these on a single page is very resource-intensive (to the point that my browser prompts me to kill the scripts). (NetBox also exposes the documentation rendered with Redoc, but it has the same issue.) This is something I was able to address using DRF's native docs, but that doesn't necessarily exclude drf-yasg.
At this point I genuinely don't know yet what the best path forward is for NetBox. If we can hammer out the current issues, I don't see a need to move away from drf-yasg. At any rate, I want to ensure that we maintain the OpenAPI specification generation, which is relied upon by other tools such as go-netbox. I'm very open to any suggestions you or anyone else has.
As an example, introspection is currently broken for many (all?) SerializerMethodFields.
The best you could do here is manually annotate the methods with their expected return types: https://drf-yasg.readthedocs.io/en/stable/custom_spec.html#support-for-serializermethodfield. I could open a quick PR with this if you'd like.
One example: Swagger generates a single flat list of endpoints under /api/ instead of nesting them under their respective apps
I'm not seeing this when running netbox locally from the develop branch. Both ReDoc and swagger-ui group the endpoints by their first path component.
and rendering all of these on a single page is very resource-intensive (to the point that my browser prompts me to kill the scripts).
This is a real issue, with a two-sided cause:
defaultModelsExpandDepth of 3, which means the models section at the bottom is fully expanded; this is the main cause of the initial stuttering You could mitigate the first point by either
a. caching the generated schema
b. pre-rendering the schema during packaging, deployment or app startup
(NetBox also exposes the documentation rendered with Redoc, but it has the same issue.)
There is an EXPAND_RESPONSES setting for ReDoc similar to above, but it doesn't have as much of an effect - ReDoc is slower than swagger-ui in general.
You can further mitigate this by pre-rendering the whole HTML page using https://github.com/Rebilly/ReDoc/blob/master/cli/README.md.
The best you could do here is manually annotate the methods with their expected return types
Thanks, I've opened #2968 for this and will give it some more thought.
I'm not seeing this when running netbox locally from the develop branch
I ended up fixing this just recently under #2705. Turns out I just had to exclude the root API view.
Looks like we have some options for improving performance; thank you for mentioning those. I'm going to experiment and see what we can improve.
i ended up fixing this just recently under #2705. Turns out I just had to exclude the root API view.
Hmm, that shouldn't have been necessary, looks like an interesting bug. I'll look to fix it.
I think it's because of the way we use nested routers but present a custom root view at /api/. The inspection considers it an endpoint, so it groups everything under /api instead of the individual apps if the root view isn't excluded.
Yeah, but the grouping works by setting the common /api/ prefix as the OpenAPI basePath. This should work even for /api/ itself, since the root view remains as just /. So definitely a bug in drf-yasg.
It seems like we've been able to distill this issue into several more succinct components that will be easier to tackle, some of which have already been addressed. I'd like to stick with drf_yasg and continue working to iron out the remaining issues with our implementation. As such, I'm going to close this issue and remove it as a v2.6 milestone. Thank you @axnsan12 for your valuable insight!
Most helpful comment
It seems like we've been able to distill this issue into several more succinct components that will be easier to tackle, some of which have already been addressed. I'd like to stick with drf_yasg and continue working to iron out the remaining issues with our implementation. As such, I'm going to close this issue and remove it as a v2.6 milestone. Thank you @axnsan12 for your valuable insight!