Fix pylint warnings inconsistent-return-statements #1408
Conversation
tiran
force-pushed
the
inconsistent-return-statements
branch
2 times, most recently
from
6fbe9e2
to
491adfa
Compare
Add consistent return to all functions and methods that are covered by tox -e pylint[23]. I haven't checked if return None is always a good idea or if we should rather raise an error. See: https://pagure.io/freeipa/issue/7326 Signed-off-by: Christian Heimes <cheimes@redhat.com>
tiran
force-pushed
the
inconsistent-return-statements
branch
from
491adfa
to
b40ec0c
Compare
| @@ -77,3 +77,4 @@ def output_for_cli(self, textui, result, ldapuri, **options): | |||
| ldapuri) | |||
| return 1 | |||
| textui.print_plain(unicode(self.pwd_migration_msg)) | |||
| return None | |||
There was a problem hiding this comment.
cli class expects that output_for_cli returns either error code or something that evaluates into a False. In the latter case we get the result silenced (cli.run() returns 0). Probably better to change return None to return 0 for consistency.
| @@ -118,6 +118,8 @@ def encrypt(data, symmetric_key=None, public_key=None): | |||
| label=None | |||
| ) | |||
| ) | |||
| else: | |||
| return None | |||
There was a problem hiding this comment.
encrypt() cannot return None because we call base64.b64encode() on the result unconditionally.
>>> import base64
>>> base64.b64encode(None)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib64/python3.6/base64.py", line 58, in b64encode
encoded = binascii.b2a_base64(s, newline=False)
TypeError: a bytes-like object is required, not 'NoneType'
>>>
I think for both encrypt() and decrypt() we can return b'' or similar.
There was a problem hiding this comment.
No, returning b'' is wrong and will hide bugs.
We have returned None in the past. This change just makes it obvious and explicit.
There was a problem hiding this comment.
then I'd suggest to raise an error here. Since both encode and decode cannot function without symmetric or public/private keys, raising an exception is better than failing in base64.
| @@ -150,6 +152,8 @@ def decrypt(data, symmetric_key=None, private_key=None): | |||
| except ValueError: | |||
| raise errors.AuthenticationError( | |||
| message=_('Invalid credentials')) | |||
| else: | |||
| return None | |||
There was a problem hiding this comment.
Same here -- probably better to return b''
There was a problem hiding this comment.
Same here. The change just makes return None explicit.
|
@tiran will make a separate PR for the exception raising for non-intended use of |
|
master:
|
Add consistent return to all functions and methods that are covered by
tox -e pylint[23]. I haven't checked if return None is always a good
idea or if we should rather raise an error.
See: https://pagure.io/freeipa/issue/7326
Signed-off-by: Christian Heimes cheimes@redhat.com