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

Fix and reenable prance-based openapi spec validation #5709

Merged
merged 4 commits into from
Aug 19, 2022

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Aug 12, 2022

Background

I'm working towards introducing pants. One of the first things we will use it for is linting / reformatting (it will run black, flake8, etc). Validation of our openapi schema should happen more regularly, so we'll hook that up so pants runs it when it runs lint tools

While hooking that up, I noticed parts of the validation commented out. So, I searched through the git history to figure out when and why.
Basically, it's just tech debt that no one had time to address. So, this is a step in that direction.

Overview

Fix and reenable prance-based openapi spec validation, but make our custom x-api-model validation optional as the spec is out-of-date.

This also fixes issues identified by the now-enabled spec validation.

Related: #3575 #3788

@cognifloyd cognifloyd added this to the 3.8.0 milestone Aug 12, 2022
@cognifloyd cognifloyd self-assigned this Aug 12, 2022
@cognifloyd cognifloyd requested review from nzlosh, rush-skills and a team August 12, 2022 20:08
@pull-request-size pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label Aug 12, 2022
@cognifloyd cognifloyd added API bug fix pantsbuild size/S PR that changes 10-29 lines. Very easy to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Aug 12, 2022
@pull-request-size pull-request-size bot added size/M PR that changes 30-99 lines. Good size to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Aug 12, 2022
@cognifloyd cognifloyd force-pushed the openapi-validation branch 2 times, most recently from e6289fa to 8aecfa2 Compare August 12, 2022 21:12
@cognifloyd cognifloyd requested a review from a team August 15, 2022 15:28
@cognifloyd cognifloyd enabled auto-merge August 18, 2022 01:46
Copy link
Contributor

@nzlosh nzlosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

There are a lot of issues, so we're only partially validating the spec
now. We still validate with prance, but skip checking x-api-model
because there are so many legacy issues.

I looked at adding the x-api-model ... but wow, we haven't been adding
that for a long time. And there are open issues about it:
#3575
#3788

So, we just make it possible, but optional, to run the schema validation.
@cognifloyd cognifloyd merged commit d0937f9 into master Aug 19, 2022
@cognifloyd cognifloyd deleted the openapi-validation branch August 19, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API bug fix pantsbuild size/M PR that changes 30-99 lines. Good size to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants