new: Add support for loading JSON OpenAPI spec files #629
Conversation
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
lgarber-akamai
added
new-feature
lgarber-akamai
commented
Jul 24, 2024
| @contextlib.contextmanager | ||
| def _get_spec_file_reader( | ||
| spec_location: str, | ||
| ) -> ContextManager[IO]: |
Contributor
Author
There was a problem hiding this comment.
Given the new OpenAPI spec is so large, I used a ContextManager & stream here to avoid loading the spec into memory more times than we need to.
lgarber-akamai
commented
Jul 24, 2024
|
|
||
|
|
||
| @contextlib.contextmanager | ||
| def open_fixture(filename: str) -> ContextManager[TextIO]: |
Contributor
Author
There was a problem hiding this comment.
This is necessary so we can manually load raw fixture contents in unit tests
lgarber-akamai
commented
Jul 24, 2024
| assert "not a registered plugin" in msg | ||
| assert code == 14 | ||
|
|
||
| def test_bake_command_bad_website(self, capsys, mock_cli): |
Contributor
Author
There was a problem hiding this comment.
I decided to remove this error handler/test because the detailed error gives more valuable information (especially when it comes to cert issues):
Failed to load spec: HTTPSConnectionPool(host='fake.linode.com', port=443): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPSConnection object at 0x10f38a5d0>: Failed to resolve 'fake.linode.com' ([Errno 8] nodename nor servname provided, or not known)"))
lgarber-akamai
commented
Jul 24, 2024
lgarber-akamai
force-pushed
the
new/spec-target-repoint
branch
from
e4095ce to
f62c0b1
Compare
lgarber-akamai
requested review from
yec-akamai and
zliang-akamai
and removed request for
a team
Comment on lines
+234
to
+244
|
|
||
| with CLI._get_spec_file_reader(spec_location) as f: | ||
| parsed = CLI._parse_spec_file(f) | ||
|
|
||
| return OpenAPI(parsed) | ||
|
|
||
| @staticmethod | ||
| @contextlib.contextmanager | ||
| def _get_spec_file_reader( | ||
| spec_location: str, | ||
| ) -> ContextManager[IO]: |
Member
There was a problem hiding this comment.
Very cool usage of context manager!
zliang-akamai
approved these changes
Jul 26, 2024
Member
zliang-akamai
left a comment
zliang-akamai
left a comment
There was a problem hiding this comment.
Works well on my side, nice work!
yec-akamai
approved these changes
Aug 5, 2024
Contributor
yec-akamai
left a comment
yec-akamai
left a comment
There was a problem hiding this comment.
Tested locally and works great, nice work!
lgarber-akamai
removed
the
do-not-merge
zliang-akamai
approved these changes
Aug 8, 2024
lgarber-akamai
added
the
do-not-merge
lgarber-akamai
changed the title
lgarber-akamai
changed the title
lgarber-akamai
force-pushed
the
new/spec-target-repoint
branch
from
357f1fd to
dddc940
Compare
lgarber-akamai
removed
the
do-not-merge
zliang-akamai
approved these changes
Oct 25, 2024
yec-akamai
approved these changes
Oct 29, 2024
Contributor
yec-akamai
left a comment
yec-akamai
left a comment
There was a problem hiding this comment.
LGTM! Just FYI when I run the integration tests with different specs, they throw some different errors. Probably because the spec versions are different and have different change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
📝 Description
This pull request adds support for loading the Linode OpenAPI spec as either a JSON or YAML file, which will prepare the CLI for the move away from the legacy spec publication system.
NOTE: This pull request previously included the removal of the linode-api-docs latest version resolution, but will be moved to a separate PR due to the delayed move to the akamai-apis repository.
✔️ How to Test
The following test steps assume you have pulled down this PR locally.
Installing with the TechDocs OpenAPI spec (JSON):
Installing with the Legacy OpenAPI spec (YAML):
Integration Testing
Unit Testing
Manual Testing
Manual testing can be done by simply running
linode-clicommands after running one of the above installation commands.