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

Swagger/OpenAPI: Syntax error in /system/logs and LogEntry definition #1287

Closed
3 tasks done
nlochschmidt opened this issue Aug 20, 2019 · 6 comments
Closed
3 tasks done

Comments

@nlochschmidt
Copy link
Contributor

My actions before raising this issue

Expected Behaviour

When following the instructions as described in https://github.com/openfaas/faas/tree/master/api-docs, the Swagger Editor should render the API documentation without errors

Current Behaviour

Swagger editor shows this message:

swagger

Possible Solution

Fix the indentation in the description of the /system/logs endpoint and rename property to properties in the LogEntry definition

Steps to Reproduce (for bugs)

  1. Head over to the Swagger editor
  2. Now click File -> Import URL
  3. Type in https://raw.githubusercontent.com/openfaas/faas/master/api-docs/swagger.yml and click OK

Additional ideas of improvement

  • I'd suggest adding a tool such as spectral to the CI build pipeline to lint the api spec
  • I see in Story: OpenFaaS SDKs for each language #1246 that you do not plan to use the spec for generating SDKs but if other people want to do it, it would make sense to handle the warnings (such as missing operationId) as well.
@alexellis
Copy link
Member

Hi @nlochschmidt

Thank you for finding this. I can't think how we let this error slip through, but I've notified the author based upon git blame. Going forward, this error-checking would reduce the risk of errors being introduced again.

spectral appears to be a JavaScript tool and I would rather not having to mandate Node.js for development of OpenFaaS, so can we run it in a Docker container perhaps?

If you are happy to fix the error in the Swagger I will be standing by to merge it.

Thank you again for picking this up.

Alex

@nlochschmidt
Copy link
Contributor Author

Will do.

Regarding the linter: It's quite easy to run the linter in docker as there is an official docker image from Stoplight. Basically it would work like this:

docker run -it --rm -v "$APIDOC_DIR":"$APIDOC_DIR" stoplight/spectral lint "$APIDOC_DIR/swagger.yml"

The return code will be non-zero in case of errors, I haven't tried yet if warnings lead to non-zero exit code as well, but would hope not.

@nlochschmidt
Copy link
Contributor Author

As you can see I have opened an MR for the api docs. I would open a second one for the linter if I get around to understand how the ci.sh works 😉

@nlochschmidt
Copy link
Contributor Author

nlochschmidt commented Aug 21, 2019

I had a look into the OpenAPI linter "spectral" but it seems like warnings cause the same non-zero exit code (1) as errors and there is an open issue for this in spectral: stoplightio/spectral#368

Two options:
1.) fix all the warnings (see spectral-warnings.txt)
2.) exclude all those rulesets from the linter that cause warnings

@alexellis
Copy link
Member

@nichochen can this be closed now?

@nlochschmidt
Copy link
Contributor Author

The issue has been resolved.

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

No branches or pull requests

2 participants