Skip to content

Support inline object schemas #236

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

Merged
merged 33 commits into from
Nov 10, 2020
Merged

Support inline object schemas #236

merged 33 commits into from
Nov 10, 2020

Conversation

dbanty
Copy link
Collaborator

@dbanty dbanty commented Nov 6, 2020

Requested by a whole host of people, this will close #162 by generating classes for object-type schemas declared "inline" instead of just for those included in references. Today, depending on where there declared, those types of schemas are either converted to generic dictionaries or thrown out altogether.

In order to achieve this, this PR also includes some major refactoring of the way Models (classes to be generated from schemas) and Enums are handled.

TODO:

  • Pull in the new UNSET features once merged to main (Better compatibility for "required" vs. "nullable" #230)
  • Finish removing DictProp usages in favor of ModelProperty
  • Switch to attrs from dataclasses in properties code
  • Make responses utilize properties to support all the same types
  • Improve naming of inline schemas based on parent name
  • Get coverage back up
  • Add in depth CHANGELOG entry for what to expect in the new version
  • Generate some clients for manual testing

dbanty and others added 22 commits October 3, 2020 17:00
…uired and nullable. Model to_dict methods now have parameters to alter what fields are included/excluded
Co-authored-by: Dylan Anthony <43723790+dbanty@users.noreply.github.com>
Co-authored-by: Dylan Anthony <43723790+dbanty@users.noreply.github.com>
…d but nullable properties no longer have a default value
@codecov
Copy link

codecov bot commented Nov 6, 2020

Codecov Report

Merging #236 (c01a89f) into main (296ffb6) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #236   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           41        46    +5     
  Lines         1332      1302   -30     
=========================================
- Hits          1332      1302   -30     
Impacted Files Coverage Δ
openapi_python_client/__init__.py 100.00% <100.00%> (ø)
openapi_python_client/parser/openapi.py 100.00% <100.00%> (ø)
...penapi_python_client/parser/properties/__init__.py 100.00% <100.00%> (ø)
...enapi_python_client/parser/properties/converter.py 100.00% <100.00%> (ø)
...i_python_client/parser/properties/enum_property.py 100.00% <100.00%> (ø)
..._python_client/parser/properties/model_property.py 100.00% <100.00%> (ø)
...penapi_python_client/parser/properties/property.py 100.00% <100.00%> (ø)
openapi_python_client/parser/properties/schemas.py 100.00% <100.00%> (ø)
openapi_python_client/parser/responses.py 100.00% <100.00%> (ø)
... and 5 more

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 296ffb6...c01a89f. Read the comment docs.

@dbanty
Copy link
Collaborator Author

dbanty commented Nov 6, 2020

@emannguitar there is something weird going on here with Unset and json body params. Not sure why it's doing what it's doing. Looks to me like the param is required? I've been staring at this too long and could use a second set of eyes.

The problem child is in golden-record/api/tests/test_inline_objects/_get_kwargs if you get a moment and can poke at it. I think that's the last bit of failing tests before I can move on to implementing the new features for responses too.

@dbanty
Copy link
Collaborator Author

dbanty commented Nov 7, 2020

Nevermind I fixed it! Was reassigning a variable in a loop 🤦🏻‍♂️

@dbanty
Copy link
Collaborator Author

dbanty commented Nov 7, 2020

Because naming fixes are actually required to get this feature across the finish line, I'm going to close #191 with this too.

@dbanty dbanty force-pushed the support_inline_object_schemas branch from d58d0fa to f3672a1 Compare November 7, 2020 23:15
@dbanty dbanty force-pushed the support_inline_object_schemas branch from 98ce9b1 to 0cbbcd4 Compare November 8, 2020 01:06
@dbanty dbanty force-pushed the support_inline_object_schemas branch from 0cbbcd4 to fbdbd21 Compare November 8, 2020 15:18
@dbanty
Copy link
Collaborator Author

dbanty commented Nov 8, 2020

I generated a prox trace client and read through some of it, it all looked good. Probably should generate inv/frt clients and test them in FST just to make sure nothing subtle is broken.

After merge I'm going to release main as 0.7.0-alpha.0 so any adventurous souls can try out all the breaking changes before we call it done. @emannguitar will hopefully be one of those people.

@dbanty dbanty changed the title WIP - Support inline object schemas Support inline object schemas Nov 8, 2020
@dbanty dbanty requested a review from emann November 8, 2020 16:36
Copy link
Collaborator

@emann emann left a comment

Choose a reason for hiding this comment

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

Breaking change with File stuffs needs to be documented + a suggestion for possible improvement on converting defaults, but other than that looks good!

@dbanty
Copy link
Collaborator Author

dbanty commented Nov 9, 2020

@emannguitar I believe this is ready for approval now? Double check my new changelog note makes sense.

@dbanty dbanty requested a review from emann November 9, 2020 17:46
@dbanty dbanty merged commit b3f13a0 into main Nov 10, 2020
@dbanty dbanty deleted the support_inline_object_schemas branch November 10, 2020 15:42
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.

Support object schemas in responses
2 participants