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: #182. Fix yaml support. Improve OAS3 #183

Merged
merged 1 commit into from Jul 27, 2020

Conversation

ioggstream
Copy link
Contributor

This PR

Fix yaml support
Improves OAS3
Use snaphots for tests

@@ -633,10 +632,21 @@ def parse_raw_obj(self, name: str, raw: Dict[str, Any], path: List[str]) -> None
self.parse_ref(obj, path)

def parse_raw(self) -> None:
raw_obj: Dict[Any, Any] = json.loads(self.text) # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Always use yaml parser so we don't care about file format.

Copy link
Owner

Choose a reason for hiding this comment

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

Great 👍

Copy link
Contributor Author

@ioggstream ioggstream left a comment

Choose a reason for hiding this comment

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

Minor fixes and start splitting files out of the test file.

self.parse_raw_obj(key, model, [path, key])


def get_oas_schema(oas_spec: Dict[Any, Any]):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Support for different OAS schemas:
OAS2 - #/definitions
OAS3- #/components/schemas
...

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, I forgot the difference.
This file is for JSON Schema.
this method name should not have OSA.
Would you please remove OSA from the method ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just noticed you had another class for OAS3, and I moved out stuff there.
Should be fixed now.
We'll find a way to fix the usage of definitions in jsonschema another time :)

Copy link
Owner

Choose a reason for hiding this comment

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

This code generator supported only OSA3 for the first time.
But, I supported JSON Schema by a request.

We will fix it next time:wink:

@codecov
Copy link

codecov bot commented Jul 27, 2020

Codecov Report

Merging #183 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #183   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          946       947    +1     
  Branches       189       189           
=========================================
+ Hits           946       947    +1     
Impacted Files Coverage Δ
datamodel_code_generator/parser/jsonschema.py 100.00% <100.00%> (ø)
datamodel_code_generator/parser/openapi.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74e9139...8b2e529. Read the comment docs.

@koxudaxi
Copy link
Owner

@ioggstream
Thank you for creating this PR.
I reviewed it.
Otherwise ok 😺

@ioggstream
Copy link
Contributor Author

Should be ok. Take some time to review in the next days. :D

@koxudaxi
Copy link
Owner

I think this PR is fine.

I have merged this PR because I prefer a short iteration in this phase.

@koxudaxi koxudaxi merged commit f006060 into koxudaxi:master Jul 27, 2020
@koxudaxi
Copy link
Owner

@ioggstream
I have released the version as 0.5.20

Thank you very much.

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

Successfully merging this pull request may close these issues.

None yet

2 participants