Closed
Conversation
This is for us. This requires: - [ ] Dorthu/openapi3#21 - [ ] Dorthu/openapi3#24 - [ ] Dorthu/openapi3#25 Currently, the Linode CLI uses an OpenAPI parser that was written to our OpenAPI spec (at the time the CLI was written) and evolved slowly as issues were found. What's resulted is a very organic, free-range parser that exists specifically for the Linode CLI and is rather difficult to unwind and maintain. This change removes the CLI's spec parser in favor of the [openapi3 python package](https://github.com/Dorthu/openapi3), which I wrote after completing the CLI as a proper spec parser, in part to replace the one in this project. The openapi3 package [is already used to validate Linode's OpenAPI specification during automated builds](https://github.com/linode/linode-api-docs/blob/9da259cbf7f50c3d26fec1f9a227337c6dc87068/.travis.yml#L6), meaning that transitioning to this parser will ensure the doc's builds never fall out of sync with the CLI's capabilities. :warning: This is a major change, and will require manual testing. Additional considerations: - [ ] Ensure all CLI-specific spec extensions are still supported - [ ] Ensure all first-party plugins function as needed - [ ] Adjust requirements for installation as necessary - [ ] How large of a version bump should this be? (in theory it's only a refactor, but it also has a lot of potential to break things unintentionally).
Contributor
|
need to add the regarding the version change, i think given the scope it warrants a minor version bump. There's nothing inherently breaking and it would be confusing for customers to upgrade a major version bump for some under the hood changes. |
Dorthu
added a commit
that referenced
this pull request
Aug 10, 2021
Closes #255 The spec for LKE Node Pools included deeper `$ref` nesting than the CLI's parser supported. This change resolves one more level of `$ref`, allowing node pools counts to be updated easily. This isn't an awesome fix; the real problem here is the fragility of the CLI's spec parser, and the real fix is to implement a better spec parser, as #218 attempted to do. That effort should be completed to resolve all like issues that may be found in the future.
Abandoned the concept of storing the parsed OpenAPI object, as pickling it didn't work as intended. Working on restructuring code so that baked data objects are separate from runtime logic to keep the distinction between what is safe to pickle and what isn't easier, and just to improve structure overall.
Dorthu
added a commit
to Dorthu/openapi3
that referenced
this pull request
Nov 12, 2021
I found this while working on linode/linode-cli#218 As it happens, a schema fragment like this: ```yaml paths: /example: get: responses: '200': content: application/json: schema: allOf: - $ref: '#/components/schema/Something' - type: object properties: new: type: string ``` will incorrectly add the `new` property to the `components/schemas/Something` schema as it is seen by _all_ references to it.
This requires the bug fix in Dorthu/openapi3#52
Dorthu
added a commit
to Dorthu/openapi3
that referenced
this pull request
Nov 12, 2021
I found this while working on linode/linode-cli#218 For a spec fragment like this: ```yaml schema: allOf: - $ref: '#/components/schemas/Example' - type: object properties: foo: $ref: '#/components/schemas/foo' bar: type: array items: $ref: '#/components/schemas/bar' ``` neither of the `$ref`s in the second schema in the allOf will be resolved (the runtime will see each as a Reference type pointing to the correct part of the schema).
With Dorthu/openapi3#53, all endpoints now parse correctly
This is still very much a WIP, but it looks like it mostly builds correctly (save for one more openapi3 bug I found).
This still needs some work, as we're not actually flattening responses yet, but the existing code should be able to be adapted to fit the new structure pretty easily.
Dorthu
added a commit
to Dorthu/openapi3
that referenced
this pull request
Jan 10, 2022
I found this while working on linode/linode-cli#218 For a spec fragment like this: ```yaml schema: allOf: - $ref: '#/components/schemas/Example' - type: object properties: foo: $ref: '#/components/schemas/foo' bar: type: array items: $ref: '#/components/schemas/bar' ``` neither of the `$ref`s in the second schema in the allOf will be resolved (the runtime will see each as a Reference type pointing to the correct part of the schema).
Dorthu
added a commit
to Dorthu/openapi3
that referenced
this pull request
Jan 10, 2022
I found this while working on linode/linode-cli#218 As it happens, a schema fragment like this: ```yaml paths: /example: get: responses: '200': content: application/json: schema: allOf: - $ref: '#/components/schema/Something' - type: object properties: new: type: string ``` will incorrectly add the `new` property to the `components/schemas/Something` schema as it is seen by _all_ references to it.
Dorthu
added a commit
that referenced
this pull request
Mar 15, 2022
This fixes the `linode-cli databases mysql-update` command. This command accepts an array, called `--allow-list`. This is unremarkable, except that the spec nests this list under three layers of references. That's valid, but the CLI didn't handle it well. This is one of the problems that a better spec parser would likely solve for us immediately: #218 This change repeatedly resolves references in args until there are none left, handling layers of references for a single argument.
lgarber-akamai
pushed a commit
to lgarber-akamai/linode-cli
that referenced
this pull request
Oct 26, 2022
This fixes the `linode-cli databases mysql-update` command. This command accepts an array, called `--allow-list`. This is unremarkable, except that the spec nests this list under three layers of references. That's valid, but the CLI didn't handle it well. This is one of the problems that a better spec parser would likely solve for us immediately: linode#218 This change repeatedly resolves references in args until there are none left, handling layers of references for a single argument.
jriddle-linode
pushed a commit
to jriddle-linode/linode-cli
that referenced
this pull request
Apr 3, 2023
Merged
jriddle-linode
added a commit
that referenced
this pull request
Jul 3, 2023
## 📝 Description **What does this PR do and why is this change necessary?** Converting the CLI to use the OpenAPI3 Spec Parser this pr will primarily only be changing how cli.bake works and future PRs will add the rest of the features OpenAPI3 offers ## ✔️ How to Test **How do I run the relevant unit/integration tests?** `make test` heavily stolen from @Dorthu #218 --------- Co-authored-by: Jacob Riddle <jriddle@akamai.com> Co-authored-by: Lena Garber <lgarber@akamai.com> Co-authored-by: @Dorthu
Collaborator
Author
|
This was completed in #423; closing this branch |
smurfix
pushed a commit
to smurfix/openapi3
that referenced
this pull request
Nov 20, 2024
I found this while working on linode/linode-cli#218 For a spec fragment like this: ```yaml schema: allOf: - $ref: '#/components/schemas/Example' - type: object properties: foo: $ref: '#/components/schemas/foo' bar: type: array items: $ref: '#/components/schemas/bar' ``` neither of the `$ref`s in the second schema in the allOf will be resolved (the runtime will see each as a Reference type pointing to the correct part of the schema).
smurfix
pushed a commit
to smurfix/openapi3
that referenced
this pull request
Nov 20, 2024
I found this while working on linode/linode-cli#218 As it happens, a schema fragment like this: ```yaml paths: /example: get: responses: '200': content: application/json: schema: allOf: - $ref: '#/components/schema/Something' - type: object properties: new: type: string ``` will incorrectly add the `new` property to the `components/schemas/Something` schema as it is seen by _all_ references to it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is for us.
This requires:
Currently, the Linode CLI uses an OpenAPI parser that was written to our
OpenAPI spec (at the time the CLI was written) and evolved slowly as
issues were found. What's resulted is a very organic, free-range parser
that exists specifically for the Linode CLI and is rather difficult to
unwind and maintain.
This change removes the CLI's spec parser in favor of the openapi3
python package, which I wrote after
completing the CLI as a proper spec parser, in part to replace the one
in this project. The openapi3 package is already used to validate
Linode's OpenAPI specification during automated builds,
meaning that transitioning to this parser will ensure the doc's builds
never fall out of sync with the CLI's capabilities.
Additional considerations:
refactor, but it also has a lot of potential to break things
unintentionally).
Notes to self: