scrapy-plugins / scrapy-zyte-smartproxy

Zyte Smart Proxy Manager (formerly Crawlera) middleware for Scrapy
BSD 3-Clause "New" or "Revised" License
356 stars 88 forks source link

Use CONNECT method to contact Crawlera proxy #44

Closed redapple closed 6 years ago

redapple commented 7 years ago

Scrapy can connect fine with Crawlera using CONNECT method. I'm not 100% sure we need to have this on by default though. This could be switchable with a new CRAWLERA_USE_HTTP_CONNECT setting of some sort.

codecov-io commented 7 years ago

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #44     +/-   ##
=========================================
- Coverage   93.75%   93.65%   -0.1%     
=========================================
  Files           2        2             
  Lines         128      126      -2     
=========================================
- Hits          120      118      -2     
  Misses          8        8
Impacted Files Coverage Δ
scrapy_crawlera/middleware.py 93.54% <ø> (-0.11%) :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 26b82e3...4e11552. Read the comment docs.

asadurski commented 6 years ago

I think we should make the use of CONNECT configurable, perhaps through a key in _settings (https://github.com/scrapy-plugins/scrapy-crawlera/blob/master/scrapy_crawlera/middleware.py#L24)?

ghost commented 6 years ago

@eLRuLL - Sorry for accidentally requesting review there. Github recommended you as a reviewer, but I misread the UI to mean that you had recommendations. :)

eLRuLL commented 6 years ago

no problem @cathalgarvey I accept it btw 😄

dangra commented 6 years ago

@cathalgarvey master branch builds are broken since this PR was merged. Can you take a look please?

ghost commented 6 years ago

Sure @dangra