pants
pants copied to clipboard
pantsbuild
Bundle OpenAPI files as a single resource
An old branch I decided to dust off and finish. 🧹
The OpenAPI targets were a great addition to allow futher OpenAPI-related functionality in Pants (codegen, linters, etc.), but I failed to dogfood them for my own use case. My problem with the current implementation is that it's hard to use an openapi_document as a dependency of a python_source due to it behaving more or less like a file() would.
At work we currently have all our API descriptions as resource() so they're bundled with the Python code that uses them for request validation and whatnot. With openapi_document this would require you to manually specify your openapi_document and all its openapi_source dependencies as inputs to experimental_wrap_as_resources, which is annoying when the OpenAPI targets all have dependency inference.
This PR uses codegen and redocly bundle to bundle an openapi_document and its openapi_sources into a single file when the openapi_document is needed as a dependency by another target. This allow developers to keep their multi-file approach when writing the API description, existing OpenAPI backends will (should?) continue to work as-is, but when consuming it elsewhere as a dependency they now only have to care about one single OpenAPI file.
I initially wrote this code back when experimental_wrap_as_resources didn't exist and openapi_document was (in our case) unusable, which is why I made it part of the main OpenAPI backend (and thus enforced). Nowadays you can, as I mentioned, likely get work around the issue with experimental_wrap_as_resources - so maybe it's better to move this functionality into its own, opt-in, backend? That being said, resolving $refs in a multi-file approach is tricky to get right, which is why a lot of OpenAPI tools don't even bother supporting it (or behave differently), so Pants "always" passing a single file around could be seen as a good thing. 🤷
Edit: redocly bundle could also be useful as a package goal to prepare the API definition for distribution by dereferencing includes, stripping x-internal paths etc., but that's a separate feature.
Is that correct?
Yeah, pretty much. Comparing openapi_document to pex_binary is good way to think about it. An OpenAPI document can consist of one or multiple files depending on how you choose to do it, but there will always be a "main file" with the root definition of your API that (optionally) reference schemas in other files.
From an OpenAPI tooling perspective, most of them expect you to give them the path to one or multiple "main files" and then they'll resolve your references to other files and read their contents internally. That's why I decided to add the openapi_document type originally, to be able to distinguish between your source files and the main file.
re: this PR. The initial idea was to simply pass openapi_document through codegen to turn it into a resource (since I wanted it included in my pex_binary). It became somewhat more complicated once I realised that an openapi_document will depend on one (itself) or multiple openapi_sources and that I would have to turn these into resources too.
That made me think. In my case (and like many others'), the OpenAPI document is only split into multiple files for maintainability and reusability. The Python library and the other OpenAPI tooling I use don't care if it's one file or multiple at all though. All multiple files do is to (potentially) add additional headache because they have to resolve all my $ref using their flavour (are all $refs relative to the main file or is each $ref relative to the file that contains it).
So instead of dealing with having to turn dependencies and dependencies of dependencies into resources as well, I decided to go down the route of bundling them all into one file, which is normally used when distributing your OpenAPI document (since it's easier to distribute one file vs. multiple). It solves it for my resource()-like use case, the existing OpenAPI backends in Pants are still passing their tests (not even sure they're affected by this change), but it is a bit on the opinionated side - hence the question whether to have it included as part of the default OpenAPI backend or to split it out into its own, optional one.
It's also worth noting that this subsystem is, like all other NodeJSToolBase-based subsystems, affected by #20062. So it crashes more often than not depending on how many OpenAPI documents Pants concurrently tries to bundle.
Sorry for letting this one sit for so long.
When you have a chance, please merge main (or rebase onto it) and add some release notes to docs/notes/2.22.x.md (maybe in a OpenAPI section). See https://github.com/pantsbuild/pants/discussions/20888 for more info.
Other than that, I think this is probably good to go from my perspective (but I'll page it all back in and do a re-review once that's updated)/
