Update of pycharts #15
Conversation
|
@VivienCabannes thank you for contributing. We will review this. |
There was a problem hiding this comment.
@apb514 @VivienCabannes can you fix the tests for this?
if you run python tests.py you will see 10 of them are failing.
| class PyChartsRequestForbiddenException(PyChartsRequestException): | ||
|
|
||
| def __init__(self, error_message=None): | ||
| self.error_message = "The server refuses to authorize the request." |
There was a problem hiding this comment.
| self.error_message = "The server refuses to authorize the request." | |
| self.error_message = 'The server refuses to authorize the request.' |
There was a problem hiding this comment.
Thanks for pointing this out, this is due to the fact that we used requests instead of urllib (which we have done for caching purposes). I would have to rewrite the tests file in order to mock requests rather than mocking urlib's urlopen function, are you okay with that?
There was a problem hiding this comment.
@VivienCabannes Yeah that would be much appreciated as we no longer need to support 2.7
There was a problem hiding this comment.
Thanks for your quick answers, I have looked a little bit about it, but that would take me too much time to implement the changes (about 90 minutes I believe), and as I am busy those days, I am afraid I will not be able to work on it at the moment.
I suggests two solutions:
- either you only merge my commit "57385c5", and you update the url in the test file accordingly.
- either you find someone to reimplement the unit test by mocking requests rather than urllib.
I hope this help, sorry I do not have some much time to work on it.
I might come back on it later if no one has fixed it.
Best regards
Vivien
Replacement of YCharts api access point.
Replacement of urllib by requests.
Handling of 403 error with requests.