scrapy / scrapyd-client

Command line client for Scrapyd server
BSD 3-Clause "New" or "Revised" License
770 stars 146 forks source link

Popen.communicate returns bytes in Python3 whereas it returns strings in Python2 #38

Closed jxltom closed 7 years ago

jxltom commented 7 years ago

The TypeError: 'str' does not support the buffer interface will raise when using Git or Mercurial version during deployment. This is because Popen.communicate returns bytes in Python3 by default whereas it returns string in Python2. As noted here, this function In Python3 will return strings if streams were opened in text mode when encoding or errors are specified or universal_newlines is true; otherwise, bytes.

This issue is fixed by passing universal_newlines=True to Popen.communicate function. In Python2, this makes all line endings will be converted to \n and function still returns as strings. In Python3, this makes sure communicate returning strings in text mode and line endings will also be converted to \n.

redapple commented 7 years ago

Looks good to me. Thanks @jxltom .

I was able to reproduce the error with current master branch and Python 3.5, deploying https://github.com/scrapinghub/testspiders to a local scrapyd:

$ cat scrapy.cfg 
[settings]
default = testspiders.settings

[deploy]
url = http://localhost:6800/
project = testspiders
version = GIT

$ scrapyd-deploy
fatal: aucun nom trouvé, impossible de décrire quoi que ce soit.
Traceback (most recent call last):
  File "/home/paul/.virtualenvs/scrapyd-client.py35/bin/scrapyd-deploy", line 6, in <module>
    exec(compile(open(__file__).read(), __file__, 'exec'))
  File "/home/paul/src/scrapyd-client/scrapyd-client/scrapyd-deploy", line 292, in <module>
    main()
  File "/home/paul/src/scrapyd-client/scrapyd-client/scrapyd-deploy", line 100, in main
    version = _get_version(target, opts)
  File "/home/paul/src/scrapyd-client/scrapyd-client/scrapyd-deploy", line 184, in _get_version
    d = p.communicate()[0].strip('\n')
TypeError: a bytes-like object is required, not 'str'

And I was able to deploy with your fix:

$ scrapyd-deploy
fatal: aucun nom trouvé, impossible de décrire quoi que ce soit.
Packing version r49-master
Deploying to project "testspiders" in http://localhost:6800/addversion.json
Server response (200):
{"project": "testspiders", "node_name": "paul-SATELLITE-R830", "spiders": 9, "version": "r49-master", "status": "ok"}
jxltom commented 7 years ago

Great. I have also tested it in Python2.7 and Python3.4 in Windows and it works all fine.

redapple commented 7 years ago

Thanks for the feedback @jxltom

jxltom commented 7 years ago

Sure, glad to contribute!

redapple commented 7 years ago

By the way, I've just released scrapyd-client 1.1.0 including your change to wrap the first Python3 compatible version.

jxltom commented 7 years ago

That's nice! I am going to upgrade.