search

averbis / averbis-python-api

Conveniently access the REST API of Averbis products using Python
Apache License 2.0
12 stars 4 forks source link

Issue #73: adding collection_process_complete #74

Closed Philly-B closed 3 years ago

codecov-commenter commented 3 years ago

Codecov Report

Merging #74 (79aeb5a) into dev (30415b1) will increase coverage by 0.08%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #74      +/-   ##
==========================================
+ Coverage   89.88%   89.96%   +0.08%     
==========================================
  Files          16       16              
  Lines        1493     1505      +12     
==========================================
+ Hits         1342     1354      +12     
  Misses        151      151
Impacted Files Coverage Δ
averbis/core/_rest_client.py 83.60% <100.00%> (+0.13%) :arrow_up:
tests/test_pipeline.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 30415b1...79aeb5a. Read the comment docs.

Philly-B commented 3 years ago

@reckart Do you mind having a look?

You will see that I also added a new function called __request_without_response since I had problems using __request_with_json_response. The python client seems to clear the response when the status code is 204, which it is for this call. Therefore I added the method mentioned above which just does not care about the content, however, checks that the status code is indeed 204.

reckart commented 3 years ago

Isn't the convention that all calls to the platform should return a JSON response?

Philly-B commented 3 years ago

That is indeed the convention, however, the python lib seems to somehow clear the content of the response if the status code is 204.

reckart commented 3 years ago

Said differently, collection process complete should probably return a 200 success and not a 204 no content.

Philly-B commented 3 years ago

Alright, I changed the backend to only return 200 instead of 204, it will be in the next release. Do you mind finishing the review of this issue?