-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Compat python 3.8 #4056
Compat python 3.8 #4056
Conversation
This pull request is still work in process. Test with Python 3.8.0b4 in my local environment. run
Most errors are due to Python 3.8 adds empty slots to abstract We can not do something like |
Codecov Report
@@ Coverage Diff @@
## master #4056 +/- ##
==========================================
- Coverage 97.77% 97.72% -0.05%
==========================================
Files 43 43
Lines 8760 8763 +3
Branches 1373 1374 +1
==========================================
- Hits 8565 8564 -1
- Misses 82 85 +3
- Partials 113 114 +1
Continue to review full report at Codecov.
|
87bf471
to
a6bb491
Compare
a6bb491
to
d1e8091
Compare
I don't know the difference between https://travis-ci.com/Hanaasagi/aiohttp/builds/127773295 and https://travis-ci.com/aio-libs/aiohttp/builds/127770827. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
Please fix my note
aiohttp/web_protocol.py
Outdated
@@ -137,7 +137,7 @@ class RequestHandler(BaseProtocol): | |||
'_waiter', '_error_handler', '_task_handler', | |||
'_upgrade', '_payload_parser', '_request_parser', | |||
'_reading_paused', 'logger', 'access_log', | |||
'access_logger', '_close', '_force_close') | |||
'access_logger', '_close', '_force_close', 'handle_request') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks irrelevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are many tests mock the handle_request
(not _handle_request
), but I could not find where call the handle_request
in aiohttp. Only some docstring mentions it.
aiohttp/aiohttp/web_protocol.py
Lines 97 to 99 in dc39183
RequestHandler handles incoming HTTP request. It reads request line, | |
request headers and request payload and calls handle_request() method. | |
By default it always returns with 404 response. |
Is it an old interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, it is an excellent demonstration of mocks weakness.
handle_request
is self._manager.request_handler
I guess.
But our mocked tests didn't fail despite the API was changed.
The fix should be done in a separate PR, this not existing yet pull request is a blocker for your work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the asyncio
changes, we could not assign a value to handle_request
. Actually this block my progress. I need to fix all like
aiohttp/tests/test_web_protocol.py
Line 403 in dc39183
srv.handle_request = handle |
change above to
with mock.patch.object(web.RequestHandler, 'handle_request', create=True, new=handle):
pass
2ab60c9
to
30e202c
Compare
30e202c
to
d09d203
Compare
Thanks! |
(cherry picked from commit 6dedbca) Co-authored-by: 秋葉 <[email protected]>
(cherry picked from commit 6dedbca) Co-authored-by: 秋葉 <[email protected]>
What do these changes do?
Compat Python 3.8.
asyncio
related exceptions are moved toasyncio.exceptions
in 3.8.make_mocked_coro
asyncio
protocolsasyncio.sleep
asyncio.coroutine
asyncio.open_connection
Are there changes in behavior for the user?
I think not.
Related issue number
It will resolve #3776
Checklist
CONTRIBUTORS.txt
CHANGES
folder<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.