Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for byte string incompatibility in ResponseModelFactory.body() on py3 #143

Merged
merged 4 commits into from
Oct 6, 2016

Conversation

aljp
Copy link
Contributor

@aljp aljp commented Oct 5, 2016

I would like to fix this warning I see a lot using silk with python3.5 and django 1.9: Response to request with pk 97a4673c-ba72-485b-88cf-f3a35e583b7b has content type application/json but was unable to parse it

In the ResponseModelFactory.body(...) method it seems to hit this exception on python3 because the content variable is a byte string, not because it is an invalid json string. I have added a check to test if it is not a str type object, then if so decode the bytes.

I do not have much experience in python2/3 compatibility so a extra pair of eyes to double check would be appreciated.

@avelis
Copy link
Collaborator

avelis commented Oct 5, 2016

@aljp Is there a unit test you can add to assert that this will work as expected on Python2 & Python3?

@aljp
Copy link
Contributor Author

aljp commented Oct 5, 2016

I wrote a unit test for this but it seems that trying to test invalid json will throw an exception within the tests due to Logger.warn('Response to request with pk %s has content type %s but was unable to parse it' % (self.request.pk, content_type)) , i.e.

Traceback (most recent call last):
  File "/Users/adam/Projects/silk/project/silk/model_factory.py", line 231, in body
    body = json.dumps(json.loads(content), sort_keys=True, indent=4)
  File "/usr/local/Cellar/python3/3.5.1/Frameworks/Python.framework/Versions/3.5/lib/python3.5/json/__init__.py", line 319, in loads
    return _default_decoder.decode(s)
  File "/usr/local/Cellar/python3/3.5.1/Frameworks/Python.framework/Versions/3.5/lib/python3.5/json/decoder.py", line 339, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/local/Cellar/python3/3.5.1/Frameworks/Python.framework/Versions/3.5/lib/python3.5/json/decoder.py", line 357, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/adam/Projects/silk/project/tests/test_compat.py", line 42, in test_python3_invalid_content_compat
    body, content = factory.body()
  File "/Users/adam/Projects/silk/project/silk/model_factory.py", line 233, in body
    Logger.warn('Response to request with pk %s has content type %s but was unable to parse it' % (self.request.pk, content_type))
AttributeError: 'NoneType' object has no attribute 'pk'

@aljp
Copy link
Contributor Author

aljp commented Oct 5, 2016

In the meantime I have commented out two tests which ensure that invalid json content returns the appropriate empty string from ResponseModelFactory in python 2 and 3

After a quick read it seems I may have to instantiate a DataCollector() and mock the request on that object to get these invalid json tests to pass? I will gladly have another look tomorrow to test if that is the case.

@avelis avelis merged commit 07f5050 into jazzband:master Oct 6, 2016
@avelis
Copy link
Collaborator

avelis commented Oct 6, 2016

@aljp Thanks for writing the unit tests and the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants