On 8/8/22 17:22, Miro Tomaska wrote: > Yes, you may build python without ssl support (if ssl libraries are not > available when you build it) and so try is still necessary (they also > do that in cpython, for example in asyncio > https://github.com/python/cpython/blob/main/Lib/asyncio/sslproto.py > <https://github.com/python/cpython/blob/main/Lib/asyncio/sslproto.py>) > > > Good point on the import. I did not realize that. will do > > Miro, please, also fix flake8 errors which are failing CI: > > > python/ovs/socket_util.py:183:80: E501 line too long (80 > 79 characters) > > I thought `make check` also ran flake8. I must have misunderstood.
It does. Actually, just simple 'make' should run it. Make sure you have it installed and ./configure detected it. Also, if you're not building with --enable-Werror, you may have just missed the warning in the log? > I will push the new patch v2 shortly. Thanks > > On Mon, Aug 8, 2022 at 5:30 AM Ilya Maximets <i.maxim...@ovn.org > <mailto:i.maxim...@ovn.org>> wrote: > > On 8/8/22 12:19, Timothy Redaelli wrote: > > On Mon, 8 Aug 2022 11:42:55 +0200 > > Ilya Maximets <i.maxim...@ovn.org <mailto:i.maxim...@ovn.org>> wrote: > > > >> On 8/8/22 04:27, Miro Tomaska wrote: > >>> Python std library SSLSocket.send does not allow non-zero value for > the optional flag. > >>> > >>> pyOpenSSL was recently switched for the Python standard library ssl > module > >>> commit 68543dd523bd00f53fa7b91777b962ccb22ce679 (python: Replace > pyOpenSSL with ssl). > > This should be in a 'Fixes:' tag, i.e. > > Fixes: 68543dd523bd ("python: Replace pyOpenSSL with ssl.") > > >>> Python SSLsocket.send() does not allow non-zero optional flag and it > will explicitly > >>> raise an exception for that. pyOpenSSL did not nothing with this flag > but kept > >>> it to be compatible with socket API. > >>> https://github.com/pyca/pyopenssl/blob/main/src/OpenSSL/SSL.py#L1844 > <https://github.com/pyca/pyopenssl/blob/main/src/OpenSSL/SSL.py#L1844> > >> > >> Hi, Miro. Thanks for the patch! > >> I'm puzzled though, how come our unit tests are not all failing? > >> I'm sure we have tests for SSL connections with python. And the > >> cited commit was tested with the neutron ml2/ovn tests over SSL > >> before being accepted with no issues observed. > > > > Because this code is only executed when you have an error > > (POLLERR or POLLHUP) and so it's very unlucky you enter this path of > > the code, > > OK. Makes sense. > > > that should definitely be fixed with your patch. > > > >>> > >>> In addition, expect for ImportError is not necessary anymore as ssl > is part of > >>> the Python standard library. This type of exception should not happen. > >> > >> IIRC, if openssl is not installed in the system, import will fail. > >> Python itself doesn't contain the implementation for SSL protocols, > >> it relies on shared libraries which are not obligatory. So, we, > >> likely, still need an import check. > > > > Yes, you may build python without ssl support (if ssl libraries are not > > available when you build it) and so try is still necessary (they also > > do that in cpython, for example in asyncio > > https://github.com/python/cpython/blob/main/Lib/asyncio/sslproto.py > <https://github.com/python/cpython/blob/main/Lib/asyncio/sslproto.py>) > > > > So, please do a v2 by restoring the try: / except ImportError: part and > > I'll ack your patch. > > Miro, please, also fix flake8 errors which are failing CI: > > python/ovs/socket_util.py:183:80: E501 line too long (80 > 79 > characters) > > Also the patch name seems to be an opposite to what the patch > is actually doing. > > Best regards, Ilya Maximets. > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
