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

refactor: use pydantic class to model search params #297

Merged
merged 25 commits into from
Oct 4, 2023
Merged

Conversation

HAEKADI
Copy link
Contributor

@HAEKADI HAEKADI commented Sep 11, 2023

Since Pydantic classes have been used to refactor the response model, this PR uses Pydantic V2 and its validator methods to parse and validate API requests.

@HAEKADI HAEKADI self-assigned this Sep 11, 2023
@HAEKADI HAEKADI marked this pull request as draft September 14, 2023 08:07
@HAEKADI HAEKADI requested a review from XavierJp September 28, 2023 15:30
@HAEKADI HAEKADI marked this pull request as ready for review September 28, 2023 15:30
aio/aio-proxy/aio_proxy/request/field_values.py Outdated Show resolved Hide resolved
aio/aio-proxy/aio_proxy/request/helpers.py Outdated Show resolved Hide resolved
aio/aio-proxy/aio_proxy/request/search_params_builder.py Outdated Show resolved Hide resolved
@XavierJp
Copy link
Contributor

XavierJp commented Oct 3, 2023

Could be nice to raise a custom Error that implement TypeError and that is catched and raised as a 400 instead of the default catchall (ValueError, TypeErorr) that might lead to an incorrect 400

@HAEKADI HAEKADI requested a review from XavierJp October 4, 2023 10:28
@HAEKADI HAEKADI requested a review from johangirod October 4, 2023 10:42
@HAEKADI
Copy link
Contributor Author

HAEKADI commented Oct 4, 2023

@johangirod I know Python is not your main language. But any feedback you have on this restructuring PR is more than welcome.

Copy link
Contributor

@XavierJp XavierJp left a comment

Choose a reason for hiding this comment

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

I love it !

@HAEKADI HAEKADI merged commit 0771ffb into main Oct 4, 2023
@HAEKADI HAEKADI deleted the params-w/-pydantic branch October 4, 2023 13:38
@johangirod
Copy link

@johangirod I know Python is not your main language. But any feedback you have on this restructuring PR is more than welcome.

Well, I went thought it, but I must admit, I was quite lost between the language and also the project architecture, which I know nothing of. I like the diff count thought 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants