Refactored for Python3 compatibility #15
Conversation
scrapy_jsonrpc/jsonrpc.py
Outdated
|
|
||
| try: | ||
| req = json_decoder.decode(jsonrpc_request) | ||
| req = json_decoder.decode(jsonrpc_request.decode()) |
There was a problem hiding this comment.
.decode() is almost always wrong - it decodes using sys.getdefaultencoding(), which can vary across machines, and which is often ascii. I think it is better to decode from UTF-8 because this is what JSON standard requires.
candale
force-pushed
the
python3_compatible
branch
from
bee6473 to
259162f
Compare
candale
force-pushed
the
python3_compatible
branch
from
259162f to
40d4ce3
Compare
|
I've also overridden the |
|
@kmike Any other observations on this PR? |
|
This works well for me with python3, no issues encountered |
|
Can we merge it? |
|
Hi @ChiraMircea can you guys write a test for this? It's extremely hard to reason on your code without tests. |
dariuschira
force-pushed
the
python3_compatible
branch
from
6fe00bd to
13fe000
Compare
|
Hey @sibiryakov I added a few basic tests and py37 to tox, hope it's enough, please let me know if you think more tests are required. |
4 participants
Refactored the code so that is Python3 compatible. It mostly had trouble when dealing with bytes and returning keys from dictionaries.
I also added
objectin the inheritance forJsonResourceandWebservicebecause their base classes did not do, which required the call to the unbound__init__method of the parent.This should solve, at least partially, issue #12 .