-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: creation of new openapi:reduce
command to reduce large OpenAPI definitions
#572
Conversation
oas-reducer
CLI into a new openapi:reduce
commandopenapi:reduce
command to reduce large OpenAPI definitions
Object { | ||
"description": "Alias for \`rdme openapi\`. [deprecated]", | ||
"name": "swagger", | ||
"position": 2, |
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.
I'm surprised that this has always been showing up after validate
in --help
with them having the same position.
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.
:lots-of-yikes:
@@ -1,4 +1,4 @@ | |||
import type { CommandOptions } from '../lib/baseCommand'; | |||
import type { CommandOptions } from '../../lib/baseCommand'; |
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.
Moved this into src/cmds/openapi/index.ts
because the openapi
command group is a family now. :corona: :vindiesel:
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.
ad0e962
to
94e982c
Compare
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.
Object { | ||
"description": "Alias for \`rdme openapi\`. [deprecated]", | ||
"name": "swagger", | ||
"position": 2, |
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.
:lots-of-yikes:
@@ -1,4 +1,4 @@ | |||
import type { CommandOptions } from '../lib/baseCommand'; | |||
import type { CommandOptions } from '../../lib/baseCommand'; |
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.
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
message: 'Enter the path to save your reduced API definition to:', | ||
initial: () => { | ||
const extension = path.extname(specPath); | ||
return `${path.basename(specPath).split(extension)[0]}-reduced${extension}`; |
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.
This'll dynamically turn petstore.json
or petstore.yaml
into petstore-reduced.{ext}
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.
:nice-rgb:
spinner.succeed(`${spinner.text} done! ✅`); | ||
} catch (err) { | ||
Command.debug(`err=${err.message}`); | ||
throw err; |
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.
The reducer throws exceptions for errors (eg. if you try to filter out too much) right now so instead of trying to capture and put those in some sort of "Yikes! We couldn't reduce this because...` error I'm just letting it do its thing.
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.
tiny nit but otherwise LGTM!
.filter(tag => { | ||
// A spec may have a `name: tag` which this JSONPath query will pick up. Since that | ||
// definitely isn't a tag we want here we need to filter them out. | ||
return typeof tag === 'string'; |
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.
damn nice find... I wish there was an easy way to test this but can't really think of one 🤔
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.
Yeah I'm not sure how to test what a prompt in prompts
is rendering without factoring this work out into a separate small utility.
message: 'Enter the path to save your reduced API definition to:', | ||
initial: () => { | ||
const extension = path.extname(specPath); | ||
return `${path.basename(specPath).split(extension)[0]}-reduced${extension}`; |
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.
:nice-rgb:
Co-authored-by: Kanad Gupta <[email protected]>
🧰 Changes
Earlier this week I ported the reduction functionality in oas-reducer, a rad library I built last year to reduce an OpenAPI definition, over into oas with the intention of porting the CLI side of it in here as a new command. This is that! Why?
oas-reducer
is cool as hell and deserves a larger audience1.Thanks to the refactoring going on in #570 developing and testing this was a goddamned dream.
🧬 QA & Testing
Here's what it looks like when you supply it a spec2:
Screen.Recording.2022-08-18.at.11.03.07.PM.mov
And with the work that's been happening with the
promptOas
library, and its file selection work, it looks great when you don't give it a spec:Screen.Recording.2022-08-18.at.11.08.56.PM.mov
Footnotes
It especially deserves to be core to
rdme
and the wholerdme openapi
flow if CX or Sales occasionally recommend it to customers to chunk up huge OAS files into smaller ones so they can be uploaded within our smol Heroku timeouts. ↩The error at the end of this video is because I didn't select a method to filter down that path by. I've since fixed this by adding a
min: 1
to themethods
question. ↩