Make 3.7 compatible, fix missing import, update to aiohttp 3 compatibility #26
Conversation
timothycrosley
left a comment
timothycrosley
left a comment
There was a problem hiding this comment.
Overall this looks great! Thanks for the work on this, there is a couple of minor things, that once resolved I'll merge this in to the main code base and do a release.
~Timothy
|
|
||
| return self | ||
|
|
||
| async def __aiter__(self): |
There was a problem hiding this comment.
looks like this change was made alluded to in 3.6 and made permanent in 3.7: https://bugs.python.org/issue27243, as annoying as it is we'll need to maintain backward compatibility for the time being.
There was a problem hiding this comment.
Right, this is the backwards compatible approach, since this will work on 3.5.3, 3.6 and 3.7 (I just verified). Whereas, the current master is not compatible with 3.7.
There was a problem hiding this comment.
It looks like I was wrong and they actually made the change in 3.5.2, that does mean that backward compatibility would be lost for the 3.5.0 and 3.5.1, however as 3.5.6 is now out I think it's safe to simply update the minimal version to 3.5.2. Thanks for verifying!
|
|
||
| from domaintools.base_results import Results | ||
|
|
||
| from domaintools.exceptions import ServiceUnavailableException |
There was a problem hiding this comment.
I apologize, I lost this comment from my initial review: I may be missing something, but I don't currently see this import used. I just want to ensure this doesn't allude to any additional work you would like to include in this pull request
|
Thank you for your patience in this review. I've had time to give it a second read over, and everything looks great! I'll be doing a release to include this work sometime this week. Thanks! ~Timothy |
|
Thank you for your patience in this review. I've had time to give it a second read over, and everything looks great! I'll be doing a release to include this work sometime this week. Thanks! ~Timothy |
3 similar comments
|
Thank you for your patience in this review. I've had time to give it a second read over, and everything looks great! I'll be doing a release to include this work sometime this week. Thanks! ~Timothy |
|
Thank you for your patience in this review. I've had time to give it a second read over, and everything looks great! I'll be doing a release to include this work sometime this week. Thanks! ~Timothy |
|
Thank you for your patience in this review. I've had time to give it a second read over, and everything looks great! I'll be doing a release to include this work sometime this week. Thanks! ~Timothy |
|
Sorry for the repeat comments on this pull request. I tried making them, unwittingly, during GitHub's outage yesterday, and while it said it had failed, it apparently queued them up to all send at once! |
…ility (#26) * Make 3.7 compatible, fix missing import, update to aiohttp 3 compat * Fix setup.py aiohttp version
3 participants
Sorry, I had to de-genericize AIter due to having to defer actually getting the results until the first async context, namely
__anext__.