Skip to content
This repository has been archived by the owner on Nov 23, 2017. It is now read-only.

Fix for #404 #460

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix for #404 #460

wants to merge 1 commit into from

Conversation

asvetlov
Copy link

Don't pass ssl certivicate error to loop.call_exception_handler()
Handle ssl.CertificateError only because ssl.SSLError may be raised not only on connection establishment.

@1st1 please take a look

@asvetlov
Fix for #404
970d993 
Don't pass ssl certivicate error to loop.call_exception_handler()
Handle ssl.CertificateError only because ssl.SSLError may be raised not only on connection establishment.
@asvetlov
Copy link
Author

Fixes #404

1st1
1st1 suggested changes Nov 14, 2016
View reviewed changes
Copy link
Member

@1st1 1st1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating _FATAL_ERROR_IGNORE worries me: I don't think I understand all consequences of this PR. Could you please elaborate more?

@asvetlov
Copy link
Author

_FATAL_ERROR_IGNORE is a tuple of exceptions which doesn't generate noisy loop.call_exception_handler() if connection cannot be established (see https://github.com/python/asyncio/blob/master/asyncio/selector_events.py#L633) for usage example.
ssl.CertificateError goes to this category -- it could be raised only at connecting stage.
The exception still is thrown to outer code but not logged unless debug mode is disabled.

@1st1
Copy link
Member

1st1 commented Nov 15, 2016

Makes sense. Would you be able to write a unittest for this?

@oherrala oherrala mentioned this pull request Jan 8, 2017
Open
@1st1
Copy link
Member

1st1 commented Mar 2, 2017

@asvetlov If you want this in 3.6.1 you need to add a unittest and resubmit this PR to python/cpython today.

@AraHaan
Copy link

AraHaan commented Mar 2, 2017 •
edited

@1st1 ok, so 3.6.1 is going out after today? btw would it also include the latest version of zlib as well because it has updated a to version above 1.2.8 this year to fix bugs and stuff.

@1st1
Copy link
Member

1st1 commented Mar 2, 2017

@1st1 ok, so 3.6.1 is going out after today? btw would it also include the latest version of zlib as well because it has updated a to version above 1.2.8 this year to fix bugs and stuff.

Yes. Re zlib - not sure, please check on GH or bugs.python.org or python-dev mailing list.

@AraHaan
Copy link

AraHaan commented Mar 3, 2017 •
edited

hmm, from the looks on http://svn.python.org/projects/external/ I only see it having zlib v1.2.3 But I could have sworn python 3.6.0 was using 1.2.8. Will look at bugs.python.org or the python-dev irc channel.

@AraHaan
Copy link

AraHaan commented Nov 19, 2017

@1st1 It looks like this repo was not marked as Read Only in it's settings to disable (I think it disables) the new issue / Pull Request buttons. Might want to do it explicitly in case someone does not read the readme in the redirect branch.

@decaz
Copy link

decaz commented Nov 22, 2017

@asvetlov are you going to resubmit this PR to python/cpython ? I hope it's not abandoned.

@LiraNuna LiraNuna mentioned this pull request Dec 3, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@1st1 1st1 1st1 requested changes

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@asvetlov @1st1 @AraHaan @decaz @brettcannon @the-knights-who-say-ni