>
> 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)
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.
I will push the new patch v2 shortly. Thanks
On Mon, Aug 8, 2022 at 5:30 AM Ilya Maximets <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> 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
> >>
> >> 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)
> >
> > 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
