feat(event_handler): add support for Pydantic models in Query and Header types#7253
Conversation
d584a8d to
f9141de
Compare
|
Hey @tonnico ! Thanks for opening this PR, we'll review it and get back to you |
|
Hey @tonnico thanks a lot for working on this. I see we have some errors in the CI, can you fix them please and then we can start reviewing the code? |
|
I apologize to overlook I still see some improvements and will fix the remaining issues. |
131310e to
d3b46be
Compare
|
@leandrodamascena I fixed code smell and the remaining typing issues. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7253 +/- ##
===========================================
+ Coverage 96.42% 96.45% +0.03%
===========================================
Files 275 275
Lines 13055 13115 +60
Branches 974 987 +13
===========================================
+ Hits 12588 12650 +62
+ Misses 362 360 -2
Partials 105 105 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks @leandrodamascena for the good starting point from your initial PR. I just fixed the merge conflict. |
|
Reviewing this now. |
|
leandrodamascena
left a comment
There was a problem hiding this comment.
Hi @tonnico! Thank you so much for your patience while waiting for my final review. I went through 4 or 5 rounds of review. I tried to create a scenario where I could break the code/functionality, and fortunately, I couldn't. I ran e2e tests with this in different scenarios.
This code is complex. You've made some important refactorings, and I can think of two or three different ways to do this implementation with the current codebase. But, to be honest, I don't know if I could do it better than you did here. The abstraction you create for some functions, the refactoring to reduce complexity, and the clarity of the code are very good. I hope I'm not missing any major potential problem I haven't seen, but our current tests are passing and that brings more peace.
That's really a super PR, thanks a lot. I sent a commit with some changes in the doc and we are ready to merge tomorrow or Friday.
THANKS A LOT! APPROVED!!!!!
|
Wow, thanks so much for your review and the nice feedback! Appreciate all the time you spent testing and checking different scenarios. Glad it held up. Looking forward to the merge! |
|
|
|
Merge conflicts resolved after upstream PRs aws-powertools#7227 and aws-powertools#7253: Core Changes: - Updated File class rename from File -> _File with public File alias - Fixed Union import for _resolve_field_type function - Removed unused _File import in dependant.py - Maintained all UploadFile and validation functionality Testing: - All 25 comprehensive tests passing - 96.36%+ codecov coverage maintained - Code quality checks passing (make format && make pr) Compatibility: - Preserves public File API for backward compatibility - Maintains UploadFile OpenAPI schema generation - All validation middleware features intact



Issue number: closes #7039
closes #5325
Summary
Changes
You can use pydantic BaseModel for Query and Header.
User experience
Grouping Query or Header params in one
BaseModel.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.