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

chore: simplify binder APIs by dropping support for generic field extractors #90

Merged
merged 10 commits into from Mar 24, 2022

Conversation

adriangb
Copy link
Owner

@adriangb adriangb commented Mar 23, 2022

BREAKING_CHANGE: removed ExtractField and ExtractRepeatedField. Extracting JSON from a form field is no longer supported. Changes to generated OpenAPI.

I think proving out the idea of very generic extractors (e.g. FromRepeatedFormField[FromJson[Model]]) was interesting. It also helped shape some APIs. But I don't think this is a feature I would use all that often, and so I would prefer to get some real world use cases form users before exposing these sorts of APIs. We can add these APIs in the future, but we can't remove them (without breaking changes).

Removing this functionality (and refactoring the binder APIs and implementations) we can arrive at a much simpler system (2 interfaces / ~2 functions to implement the binder API), and reduce the package's codebase by ~15%.

There's a lot more cleanup that needs to be done with tests after this (e.g. coverage for the file extractor); that will be done in subsequent changes.

@adriangb adriangb changed the title chore: simplify binder APIs and drop support for niche features chore: simplify binder APIs by dropping support for generic field extractors Mar 24, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2022

Codecov Report

Merging #90 (dcb8871) into main (38b5960) will increase coverage by 0.34%.
The diff coverage is 97.04%.

@@            Coverage Diff             @@
##             main      #90      +/-   ##
==========================================
+ Coverage   97.66%   98.00%   +0.34%     
==========================================
  Files         154      139      -15     
  Lines        4708     4463     -245     
  Branches      754      697      -57     
==========================================
- Hits         4598     4374     -224     
+ Misses         65       48      -17     
+ Partials       45       41       -4     
Impacted Files Coverage Δ
...advanced/additional_responses/test_tutorial_002.py 100.00% <ø> (ø)
...advanced/additional_responses/test_tutorial_003.py 100.00% <ø> (ø)
tests/test_docs/advanced/test_root_path.py 100.00% <ø> (ø)
tests/test_docs/tutorial/body/test_tutorial_001.py 100.00% <ø> (ø)
tests/test_docs/tutorial/body/test_tutorial_002.py 100.00% <ø> (ø)
tests/test_docs/tutorial/body/test_tutorial_004.py 100.00% <ø> (ø)
tests/test_docs/tutorial/body/test_tutorial_005.py 100.00% <ø> (ø)
...t_docs/tutorial/cookie_params/test_tutorial_001.py 100.00% <ø> (ø)
...ests/test_docs/tutorial/files/test_tutorial_003.py 100.00% <ø> (ø)
...ests/test_docs/tutorial/files/test_tutorial_004.py 100.00% <ø> (ø)
... and 66 more

@adriangb adriangb merged commit 1c25801 into main Mar 24, 2022
@adriangb adriangb deleted the simplify-extractors branch March 24, 2022 19:17
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