diff mbox series

[ovs-dev] tests/mfex: Don't require python cryptography.

Message ID 20230830081633.2167017-1-david.marchand@redhat.com
State Accepted
Commit 13b874f4fe89cc72e702f91183f678872ee6e88d
Headers show
Series [ovs-dev] tests/mfex: Don't require python cryptography. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

David Marchand Aug. 30, 2023, 8:16 a.m. UTC
Tests using mfex_fuzzy.py will fail on systems lacking the Python
cryptography library.
cryptography is not required, it is only imported in mfex_fuzzy.py to
silence some warnings when scapy tries to load some libraries.

Fixes: c3ed0bf34b8a ("tests/mfex: Silence Blowfish/CAST5 deprecation warnings.")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 tests/mfex_fuzzy.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Eelco Chaudron Aug. 30, 2023, 8:23 a.m. UTC | #1
On 30 Aug 2023, at 10:16, David Marchand wrote:

> Tests using mfex_fuzzy.py will fail on systems lacking the Python
> cryptography library.
> cryptography is not required, it is only imported in mfex_fuzzy.py to
> silence some warnings when scapy tries to load some libraries.
>
> Fixes: c3ed0bf34b8a ("tests/mfex: Silence Blowfish/CAST5 deprecation warnings.")
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Thanks for fixing this, verified with and without the lib installed (and flake8)!

Cheers,

Eelco

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Robin Jarry Aug. 30, 2023, 9:03 a.m. UTC | #2
David Marchand, Aug 30, 2023 at 10:16:
> Tests using mfex_fuzzy.py will fail on systems lacking the Python
> cryptography library.
> cryptography is not required, it is only imported in mfex_fuzzy.py to
> silence some warnings when scapy tries to load some libraries.
>
> Fixes: c3ed0bf34b8a ("tests/mfex: Silence Blowfish/CAST5 deprecation warnings.")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  tests/mfex_fuzzy.py | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tests/mfex_fuzzy.py b/tests/mfex_fuzzy.py
> index 30028ba7a0..50b9870641 100755
> --- a/tests/mfex_fuzzy.py
> +++ b/tests/mfex_fuzzy.py
> @@ -3,12 +3,15 @@
>  import sys
>  import warnings
>  
> -from cryptography.utils import CryptographyDeprecationWarning
> -warnings.filterwarnings(
> -    "ignore",
> -    category=CryptographyDeprecationWarning,
> -    message=r"(blowfish|cast5)",
> -)
> +try:
> +    from cryptography.utils import CryptographyDeprecationWarning
> +    warnings.filterwarnings(
> +        "ignore",
> +        category=CryptographyDeprecationWarning,
> +        message=r"(blowfish|cast5)",
> +    )
> +except ModuleNotFoundError:
> +    pass

Hi David,

Since CryptographyDeprecationWarning is a subclass of UserWarning, you
can also that simpler form:

warnings.filterwarnings("ignore", r"(blowfish|cast5)", UserWarning)
David Marchand Aug. 30, 2023, 9:11 a.m. UTC | #3
On Wed, Aug 30, 2023 at 11:03 AM Robin Jarry <rjarry@redhat.com> wrote:
>
> David Marchand, Aug 30, 2023 at 10:16:
> > Tests using mfex_fuzzy.py will fail on systems lacking the Python
> > cryptography library.
> > cryptography is not required, it is only imported in mfex_fuzzy.py to
> > silence some warnings when scapy tries to load some libraries.
> >
> > Fixes: c3ed0bf34b8a ("tests/mfex: Silence Blowfish/CAST5 deprecation warnings.")
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  tests/mfex_fuzzy.py | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/tests/mfex_fuzzy.py b/tests/mfex_fuzzy.py
> > index 30028ba7a0..50b9870641 100755
> > --- a/tests/mfex_fuzzy.py
> > +++ b/tests/mfex_fuzzy.py
> > @@ -3,12 +3,15 @@
> >  import sys
> >  import warnings
> >
> > -from cryptography.utils import CryptographyDeprecationWarning
> > -warnings.filterwarnings(
> > -    "ignore",
> > -    category=CryptographyDeprecationWarning,
> > -    message=r"(blowfish|cast5)",
> > -)
> > +try:
> > +    from cryptography.utils import CryptographyDeprecationWarning
> > +    warnings.filterwarnings(
> > +        "ignore",
> > +        category=CryptographyDeprecationWarning,
> > +        message=r"(blowfish|cast5)",
> > +    )
> > +except ModuleNotFoundError:
> > +    pass
>
> Hi David,
>
> Since CryptographyDeprecationWarning is a subclass of UserWarning, you
> can also that simpler form:
>
> warnings.filterwarnings("ignore", r"(blowfish|cast5)", UserWarning)

It means waiving more warnings than before, but I don't think it
really matters for this script.
No strong opinion against your proposal.
Robin Jarry Aug. 30, 2023, 9:13 a.m. UTC | #4
David Marchand, Aug 30, 2023 at 11:11:
> > warnings.filterwarnings("ignore", r"(blowfish|cast5)", UserWarning)
>
> It means waiving more warnings than before, but I don't think it
> really matters for this script.
> No strong opinion against your proposal.

This is very unlikely that any code will trigger a UserWarning with that
text content.
Eelco Chaudron Aug. 30, 2023, 9:21 a.m. UTC | #5
On 30 Aug 2023, at 11:13, Robin Jarry wrote:

> David Marchand, Aug 30, 2023 at 11:11:
>>> warnings.filterwarnings("ignore", r"(blowfish|cast5)", UserWarning)
>>
>> It means waiving more warnings than before, but I don't think it
>> really matters for this script.
>> No strong opinion against your proposal.
>
> This is very unlikely that any code will trigger a UserWarning with that
> text content.

Well you never know ;) I would prefer the original one, as it’s more strict.

//Eelco
Robin Jarry Aug. 30, 2023, 9:27 a.m. UTC | #6
Eelco Chaudron, Aug 30, 2023 at 11:21:
> > This is very unlikely that any code will trigger a UserWarning with that
> > text content.
>
> Well you never know ;) I would prefer the original one, as it’s more strict.

Fair enough :)
Ilya Maximets Aug. 30, 2023, 8:15 p.m. UTC | #7
On 8/30/23 10:23, Eelco Chaudron wrote:
> 
> 
> On 30 Aug 2023, at 10:16, David Marchand wrote:
> 
>> Tests using mfex_fuzzy.py will fail on systems lacking the Python
>> cryptography library.
>> cryptography is not required, it is only imported in mfex_fuzzy.py to
>> silence some warnings when scapy tries to load some libraries.
>>
>> Fixes: c3ed0bf34b8a ("tests/mfex: Silence Blowfish/CAST5 deprecation warnings.")
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
> 
> Thanks for fixing this, verified with and without the lib installed (and flake8)!
> 
> Cheers,
> 
> Eelco
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>

Thanks!  Applied and backported down to 3.2.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/tests/mfex_fuzzy.py b/tests/mfex_fuzzy.py
index 30028ba7a0..50b9870641 100755
--- a/tests/mfex_fuzzy.py
+++ b/tests/mfex_fuzzy.py
@@ -3,12 +3,15 @@ 
 import sys
 import warnings
 
-from cryptography.utils import CryptographyDeprecationWarning
-warnings.filterwarnings(
-    "ignore",
-    category=CryptographyDeprecationWarning,
-    message=r"(blowfish|cast5)",
-)
+try:
+    from cryptography.utils import CryptographyDeprecationWarning
+    warnings.filterwarnings(
+        "ignore",
+        category=CryptographyDeprecationWarning,
+        message=r"(blowfish|cast5)",
+    )
+except ModuleNotFoundError:
+    pass
 
 # flake8: noqa: E402
 from scapy.all import RandMAC, RandIP, PcapWriter, RandIP6, RandShort, fuzz