scrapy-splash icon indicating copy to clipboard operation
scrapy-splash copied to clipboard

Do not try to decode body of Response

Open whalebot-helmsman opened this issue 6 years ago • 3 comments

After discussion in https://github.com/scrapy-plugins/scrapy-splash/pull/173 Decoding body of Response using only utf-8 is right way to fail. It is better to skip decoding at all.

There is two problems with integration tests at this moment - https://github.com/scrapy/scrapy/pull/3210#issuecomment-380840192 and https://github.com/scrapy/scrapy/pull/3210#issuecomment-380840192 . Considering this, no integration tests added

Fixing Integration tests requires effort separate from proposed changes

whalebot-helmsman avatar Apr 16 '18 10:04 whalebot-helmsman

Codecov Report

Merging #175 into master will decrease coverage by 0.01%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #175      +/-   ##
==========================================
- Coverage   92.67%   92.66%   -0.02%     
==========================================
  Files           9        9              
  Lines         587      586       -1     
  Branches      118      118              
==========================================
- Hits          544      543       -1     
  Misses         21       21              
  Partials       22       22
Impacted Files Coverage Δ
scrapy_splash/response.py 94% <ø> (-0.06%) :arrow_down:

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 e40ca4f...b772360. Read the comment docs.

codecov[bot] avatar Apr 16 '18 10:04 codecov[bot]

@whalebot-helmsman I’ve found this pull request while reviewing some old pull requests. What would the next steps be for this request?

Gallaecio avatar Aug 05 '19 15:08 Gallaecio

I think we should merge it. Any thoughts @kmike ?

whalebot-helmsman avatar Aug 06 '19 05:08 whalebot-helmsman