daniellawrence
Python 3 support fix on _send method
Running a Django v1.10 app with Python v3.4.3, the _send method breaks.
Stack trace:
File "/home/pablo/.virtualenvs/test/local/lib/python3.4/site-packages/graphitesend/graphitesend.py", line 291, in _send
self.socket.sendall(message.encode("ascii"))
AttributeError: 'bytes' object has no attribute 'encode'
...
graphitesend.graphitesend.GraphiteSendException: Unknown error while trying to send data down socket to ('172.17.0.2', 8000), error: 'bytes' object has no attribute 'encode'
@daniellawrence travis errors are not from the commit i made, can you give me a hand? Thanks in advance!
Coverage decreased (-0.3%) to 93.08% when pulling 634ba684f012e56e974e12fc4106f290cccbdda5 on PabloLefort:master into 6f84c56ad4e7594cf36b0c3d4aeaa86b4dd3ef06 on daniellawrence:master.
@PabloLefort I fixed the issue you were facing with the tests. However, the coverage decrease with your commit so could you please add a test to make it at least the same as now.
Also, please note that an other ongoing PR (#70) is trying to fix a Python 3 issue and I think it's the same issue. Can you please explain what was the issue you were facing ? Our Python 3 test suite is passing so I don't understand what could be the error.
I like the fact that the PR #70 is regardless of the python version. But I also like the simplicity of your PR. You could still make it simpler by using an and clause rather than two if statements.
@Shir0kamii thanks for the tests! Yes, i can make a test for the coverage. I see the PR #70 but as you say, i also think this is a more simple fix.
The problem was that sending a test metric graphite_client.send('testing', 10) throws the stack trace of above.
Your suggest is even more simpler! I'll update it.
Coverage decreased (-0.3%) to 93.127% when pulling ffb03217e6a59892d2e2e09ebcf9745badf589db on PabloLefort:master into d18b986273488adf3db5d51de02581e927155226 on daniellawrence:master.
Coverage decreased (-0.3%) to 93.103% when pulling 961cfc65a251f69b4ed8f42450af288b656ab68d on PabloLefort:master into d18b986273488adf3db5d51de02581e927155226 on daniellawrence:master.
Coverage decreased (-0.3%) to 93.103% when pulling 961cfc65a251f69b4ed8f42450af288b656ab68d on PabloLefort:master into d18b986273488adf3db5d51de02581e927155226 on daniellawrence:master.
@PabloLefort Agreed! Once you make the coverage stage pass, I'll merge your pull request.
@Shir0kamii It could be something like this?
def test_send_values_as_bytes(self):
graphite_instance = graphitesend.init()
metric = 'metric'
if sys.version_info >= (3, 0) and type(metric) is bytes:
metric = str(metric, 'utf-8')
response = graphite_instance.send(metric, "1", "1")
response = str(response)
self.assertEqual('metric 1.000000 1' in response, True)
Thank you!
@PabloLefort I'm not sure it would increase the coverage.
Coverage is just a tool verifying that every line of code is executed. So to increase coverage you will need to make sure the test you write execute the lines of code you added. It means that you will have to send a byte object.
Also because your code will only ever be executed on Python 3, you won't be able to increase Python 2's coverage. But that's okay, I'll merge it if Python 3 pass coverage test.