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

Upgrade codegen tools in scripts/generate-open-api-spec and publish Swagger 2.0 and OpenAPI 3.0 specs #2246

Merged
merged 23 commits into from
May 30, 2023

Conversation

ff137
Copy link
Contributor

@ff137 ff137 commented May 27, 2023

Now is as good a time as any to upgrade the codegen tools in scripts/generate-open-api-spec to use the latest swaggerapi/swagger-codegen-cli release (it was ~3 years out of date).

Additionally, I added the latest openapitools/openapi-generator-cli, so that the spec generation now provides both a Swagger 2.0 spec, and an OpenApi 3.0 spec. This way users don't need to manually upgrade the spec themselves.

Previously the Swagger 2.0 spec was simply called openapi.json. This is now renamed swagger.json, with the 3.0 compatible spec in its place.

Worth noting that the spec validation still only throws warnings that can be ignored:

  • the swagger spec validation only complains about some missing operationId's (and 'host' not defined);
  • and the open-api spec validation only complains about example fields being unexpected, and a single case of attribute tags.tagsexternalDocs.url is missing.

Also, I updated the UsingOpenAPI.md file to reflect these changes, with some minor neatening up included.

Review can be validated by running scripts/generate-open-api-spec.

Discussion re further edits is welcome.

ff137 added 21 commits May 27, 2023 14:03
Signed-off-by: ff137 <[email protected]>
…6.6.0`, from previous `swaggerapi/swagger-codegen-cli:2.4.32`

Signed-off-by: ff137 <[email protected]>
Signed-off-by: ff137 <[email protected]>
Signed-off-by: ff137 <[email protected]>
Signed-off-by: ff137 <[email protected]>
Signed-off-by: ff137 <[email protected]>
@ff137 ff137 force-pushed the upgrade/swagger-codegen-cli branch from d4dc711 to 7b82ce8 Compare May 27, 2023 12:05
@ff137 ff137 changed the title Upgrade codegen tools used in scripts/generate-open-api-spec and publish Swagger 2.0 and OpenAPi 3.0 specs Upgrade codegen tools in scripts/generate-open-api-spec and publish Swagger 2.0 and OpenAPI 3.0 specs May 27, 2023
@ff137 ff137 marked this pull request as ready for review May 27, 2023 12:29
@ff137
Copy link
Contributor Author

ff137 commented May 27, 2023

@swcurran I was thinking it may be worthwhile to add an OpenAPI Spec Generation to the CI workflow. Something like:

name: OpenAPI Spec Check

on:
  push:
    branches:
      - main
  pull_request:
    branches:
      - main

jobs:
  build:
    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v2

    - name: Setup environment
      run: |
        # setup necessary environment

    - name: Generate OpenAPI specs
      run: |
        ./scripts/generate-open-api-spec

    - name: Commit changes
      run: |
        git config --local user.email "[email protected]"
        git config --local user.name "GitHub Action"
        git commit -m "Update Swagger/OpenAPI specs" -a

What do you think?

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@swcurran
Copy link
Contributor

Nice work — looks good. There sure are a lot of errors generated!!! I’m assuming that is expected/OK, given that things are working.

@jcourt562 — any thoughts on the idea of a GHA to generate the openapi JSON?

@ff137
Copy link
Contributor Author

ff137 commented May 29, 2023

@swcurran Thanks!

I would say the warnings are low priority to fix, but always worth it in the long run.

It's mainly:

  • setting operationIds everywhere
  • adding the one missing doc URL
    • it's for: description: did resolver interface.
    • I think this may be the appropriate link: https://github.com/hyperledger/aries-rfcs/tree/070b325ef4a55005d79d03eb629f0399f3a0dfc4/features/0124-did-resolution-protocol, but I may be wrong.
  • no server url being set

The UsingOpenAPI.md does say (2 years ago) aiohttp_apispec does not support adding operationId annotations via tags. Maybe that's changed.

I do have a script for patching an openapi.yml with a mapping of operation-ids. Maybe that feels like a more maintainable solution than tagging? Because then you can manage all operation id's in one file. Let me know and I can add it.

As for all the: -attribute paths ... .example is unexpected errors, I have no idea. Looking at the spec doesn't really line up with path being reported -- maybe someone else can figure that one out!

@swcurran swcurran merged commit a324357 into openwallet-foundation:main May 30, 2023
@ff137 ff137 deleted the upgrade/swagger-codegen-cli branch July 4, 2024 22:08
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