gtalarico / pyairtable

Python Api Client for Airtable
https://pyairtable.readthedocs.io
MIT License
765 stars 138 forks source link

Use `POST /listRecords` instead of `GET` when the URL gets too long #245

Closed mesozoic closed 1 year ago

mesozoic commented 1 year ago

This attempts to resolve the same issue as #222 but without as much refactoring. It was hard for me to wrap my head around everything going on in that branch, so I started by writing a unit test and an integration test and then wrote only what I needed to get them to pass. I'm open to feedback or suggestions, especially if anything else in that branch is a must-have.

codecov-commenter commented 1 year ago

Codecov Report

Merging #245 (f50c02d) into main (2f6ecd6) will increase coverage by 0.15%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #245      +/-   ##
==========================================
+ Coverage   87.34%   87.50%   +0.15%     
==========================================
  Files          14       14              
  Lines         640      648       +8     
==========================================
+ Hits          559      567       +8     
  Misses         81       81              
Impacted Files Coverage Δ
pyairtable/api/abstract.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

mesozoic commented 1 year ago

@gtalarico following our discussion, I'm going to merge bugfixes and features after about a week of being up for review if there's no open feedback. Thanks again for your support!

mesozoic commented 1 year ago

I reverted the merge; this branch does not appropriately handle all query parameters, which is something I missed porting over from the other PRs that have tried to tackle this. I'll take another pass at it after writing some more comprehensive tests.