-
Notifications
You must be signed in to change notification settings - Fork 47
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
CI - combined OpenAPI doc #87
Conversation
Looks like there is a larger problem here, the combined doc doesn't include ogcapi-features endpoints: There was a name conflict between core / (root) and ogcapi-features / (root) so I thought I was excluding / from ogcapi-features, but it ended up excluding all paths from that file. Trying to remove root from core to see if that would work I also got an error for conflicting paths |
So for now I removed the transactions extension from the complete openapi.yaml, and also the STAC core root (/) endpoint so it wouldn't conflict with the ogcapi one....although it looks like the OGC one is defined the same way. |
Oh, so now you are also struggling with the issues I was facing, too. I had hoped you had found a solution for them. I don't think there's an easy way out. The only solution I could think of could be to use the yaml-files tool again for merging the bundled files... If it's not complete, what is the benefit of the combined file? |
Well, this at least has all the most common pieces: core, features, item-search. It seems like each one of the tools has something wrong with it, some are known issues, or other people have requested the feature. swagger-combine is the closest. |
We added our |
We can solve that easily, let me issue a quick PR. Edit: It's here: #88 |
Sort, Query, ... are also included (but they have no OpenAPI Tags, which render the menu). So removing version doesn't make it "extension-free", which I thought would be a good idea... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
I'm out for the afternoon. I think we can skip the combined doc for release
and I can spend some more time figuring out the geometry collection problem
|
I think the other extensions are fairly well marked - when you see them in the openapi html it's clear they are optional extensions. So overall I like them in. My beef with version is mostly that it's 'proposal'. If we wanted a clear rule I'd say extensions get included when they are at least pilot (potentially a higher bar as we evolve).
Agreed, let's just leave it out for this release, we can mark it as something that will come in the next release. What's needed to leave it out? |
@cholmes @m-mohr I've reverted the change to the commons.yaml, and disabled the step to combine. So this is good to merge as is, there's a couple small changes, and it's using swagger-cli now instead of speccy. The problem with swagger-combine is that is So this is good to merge, @m-mohr will need to approve as he requested changes |
https://api.stacspec.org/dev/ is broken - should that load core for now? Just so that for the release there is something loading up at https://api.stacspec.org/v1.0.0-beta.1? There are links pointing to the URL. Just curious: what's the advantage of swagger-cli over speccy? |
There were some nested references due to circular dependencies that speccy didn't dereference...which is OK for the individual files, but it causes swagger-combine to break because it keeps the refs but doesn't carry over Sure, I can drop the core file under dev/ |
Okay, thank you. I'd just duplicate the index.html for now or do a redirect (we could also add this later manually after the release by commiting directly to the gh-pages branch, so if that is too cumbersome for the CI at the moment). The core folder should still resolve. |
@m-mohr ok, it will add the core openapi to the root under the version. |
Related Issue(s): #
#12
Proposed Changes:
See examples for latest published versions of this branch:
https://radiantearth.github.io/stac-api-spec/dev/
https://radiantearth.github.io/stac-api-spec/dev/core
https://radiantearth.github.io/stac-api-spec/dev/item-search
https://radiantearth.github.io/stac-api-spec/dev/ogcapi-features
https://radiantearth.github.io/stac-api-spec/dev/ogcapi-features/extensions/transaction
https://radiantearth.github.io/stac-api-spec/dev/ogcapi-features/extensions/version
Combining the files was difficult, and was a combination of arguments to bundling with swagger-combine. I had to comment out
geometrycollectionGeoJSON
in commons.yaml to get the files to build. So that entry is missing for now. I tried a few other merge tools including openapi-merge-cli and justyq
but there were issues with each of them.I think this is good enough for a beta.1, I didn't get time to update the documentation on using swagger-combine. I'd like to create an example a developer might use to merge the published files, with examples on how to exclude certain parameters or paths (this can be specified in swagger-combine config file)if they have not been implemented.