billdeitrick / pypco

A Python client for the Planning Center Online API.
MIT License
39 stars 13 forks source link

Handle API requests that return 204 #45

Closed warmach closed 1 year ago

warmach commented 3 years ago

Code to fix Issue 44 in billdeitrick/pypco

https://github.com/billdeitrick/pypco/issues/44

pastorhudson commented 2 years ago

Thanks for the PR!

Do you think we're better returning None for this or empty json {} ?

pastorhudson commented 2 years ago

Or perhaps a custom PCORequestException?

codecov[bot] commented 2 years ago

Codecov Report

Merging #45 (09534c9) into master (b23f76f) will decrease coverage by 0.50%. The diff coverage is 80.00%.

@@             Coverage Diff             @@
##            master      #45      +/-   ##
===========================================
- Coverage   100.00%   99.50%   -0.50%     
===========================================
  Files            5        5              
  Lines          200      204       +4     
===========================================
+ Hits           200      203       +3     
- Misses           0        1       +1     
Impacted Files Coverage Δ
pypco/pco.py 99.14% <80.00%> (-0.86%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

billdeitrick commented 2 years ago

@warmach Thanks for fixing this, and apologies it's taken so long to make some progress on getting it merged in! We just need to get test coverage on the change and then we'll be good to go.

@pastorhudson I think he made the right decision here with returning a null; that's a clear indication that PCO didn't send us a payload, and we don't want to throw an exception because there are legitimate calls where it looks like they return an empty payload as per #44.

billdeitrick commented 1 year ago

Fixed in 8bb4bff64d8494d36fd3e263bfb20088f46c0acc.