Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump pylint and flake8 #232

Merged
merged 5 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions knack/arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def __getattr__(self, name):

def __setattr__(self, name, value): # pylint: disable=inconsistent-return-statements
if name == 'type':
return super(CLICommandArgument, self).__setattr__(name, value)

Choose a reason for hiding this comment

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

It's a big change for our code. Can you provide more information about this.

Copy link
Member Author

@jiasli jiasli Jan 29, 2021

Choose a reason for hiding this comment

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

If we run pylint==2.6.0 on the current code, it errors out:

> tox -e py38

************* Module knack.arguments
knack\arguments.py:86:19: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:189:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:212:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:246:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:297:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.commands
knack\commands.py:283:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.deprecation
knack\deprecation.py:89:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\deprecation.py:141:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.experimental
knack\experimental.py:55:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\experimental.py:76:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.help
knack\help.py:95:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:126:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:245:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:269:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:328:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:348:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.output
knack\output.py:81:8: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.parser
knack\parser.py:112:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\parser.py:226:15: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\parser.py:262:15: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.preview
knack\preview.py:55:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\preview.py:76:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.query
knack\query.py:27:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.util
knack\util.py:45:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.testsdk.base
knack\testsdk\base.py:29:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\base.py:85:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\base.py:116:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\base.py:254:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.testsdk.checkers
knack\testsdk\checkers.py:67:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
knack\testsdk\checkers.py:80:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
knack\testsdk\checkers.py:93:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.testsdk.exceptions
knack\testsdk\exceptions.py:10:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\exceptions.py:17:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\exceptions.py:25:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)

-------------------------------------------------------------------
Your code has been rated at 9.87/10 (previous run: 10.00/10, -0.13)

The help message is pretty straight-forward:

> pylint --help-msg R1725,W0707
:super-with-arguments (R1725): *Consider using Python 3 style super() without arguments*
  Emitted when calling the super() builtin with the current class and instance.
  On Python 3 these arguments are the default and they can be omitted. This
  message belongs to the refactoring checker.

:raise-missing-from (W0707): *Consider explicitly re-raising using the 'from' keyword*
  Python 3's exception chaining means it shows the traceback of the current
  exception, but also the original exception. Not using `raise from` makes the
  traceback inaccurate, because the message implies there is a bug in the
  exception-handling code itself, which is a separate situation than wrapping
  an exception. This message belongs to the exceptions checker.

Choose a reason for hiding this comment

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

As I know, python2 does not support this, and pycharm are using the old format in code autocompletion.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have dropped Python 2 support long time ago (#174). PyCharm is only an IDE and is not the standard. We had better follow PEP 3135 instead.

Choose a reason for hiding this comment

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

We need to make the same changes for azure-cli and azure-cli-extensions after using the new knack version with this bump?

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely not. Knack itself doesn't require pylint and flake8. This PR only makes sure Knack complies with the latest pylint and flake8.

Choose a reason for hiding this comment

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

I see. pylint is still pinned to 2.3.0 in azdev.

return super().__setattr__(name, value)
self.type.settings[name] = value


Expand Down Expand Up @@ -186,7 +186,7 @@ def __call__(self, parser, namespace, values, option_string=None):
else:
namespace._argument_deprecations.append(deprecate_info) # pylint: disable=protected-access
try:
super(DeprecatedArgumentAction, self).__call__(parser, namespace, values, option_string)
super().__call__(parser, namespace, values, option_string)
except NotImplementedError:
setattr(namespace, self.dest, values)

Expand All @@ -209,7 +209,7 @@ def __call__(self, parser, namespace, values, option_string=None):
else:
namespace._argument_deprecations.append(deprecated_opt) # pylint: disable=protected-access
try:
super(DeprecatedOptionAction, self).__call__(parser, namespace, values, option_string)
super().__call__(parser, namespace, values, option_string)
except NotImplementedError:
setattr(namespace, self.dest, values)

Expand Down Expand Up @@ -243,7 +243,7 @@ def __call__(self, parser, namespace, values, option_string=None):
else:
namespace._argument_previews.append(preview_info) # pylint: disable=protected-access
try:
super(PreviewArgumentAction, self).__call__(parser, namespace, values, option_string)
super().__call__(parser, namespace, values, option_string)
except NotImplementedError:
setattr(namespace, self.dest, values)

Expand Down Expand Up @@ -294,7 +294,7 @@ def __call__(self, parser, namespace, values, option_string=None):
else:
namespace._argument_experimentals.append(experimental_info) # pylint: disable=protected-access
try:
super(ExperimentalArgumentAction, self).__call__(parser, namespace, values, option_string)
super().__call__(parser, namespace, values, option_string)
except NotImplementedError:
setattr(namespace, self.dest, values)

Expand Down
6 changes: 3 additions & 3 deletions knack/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,10 @@ def _get_op_handler(operation):
op = getattr(op, part)
if isinstance(op, types.FunctionType):
return op
# MethodType
# op as types.MethodType
return op.__func__
except (ValueError, AttributeError):
raise ValueError("The operation '{}' is invalid.".format(operation))
except (ValueError, AttributeError) as ex:
raise ValueError("The operation '{}' is invalid.".format(operation)) from ex
Comment on lines +281 to +282
Copy link
Member Author

Choose a reason for hiding this comment

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

Use Python's built-in Exception Chaining as recommended by W0707:

raise-missing-from (W0707):
Consider explicitly re-raising using the 'from' keyword Python 3's exception chaining means it shows the traceback of the current exception, but also the original exception. Not using raise from makes the traceback inaccurate, because the message implies there is a bug in the exception-handling code itself, which is a separate situation than wrapping an exception.


def deprecate(self, **kwargs):
kwargs['object_type'] = 'command group'
Expand Down
4 changes: 2 additions & 2 deletions knack/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def _default_get_message(self):
self.expiration = expiration
self._cli_version = cli_ctx.get_cli_version()

super(Deprecated, self).__init__(
super().__init__(
cli_ctx=cli_ctx,
object_type=object_type,
target=target,
Expand Down Expand Up @@ -136,4 +136,4 @@ def get_implicit_deprecation_message(self):
'tag_func': lambda _: '',
'message_func': get_implicit_deprecation_message
})
super(ImplicitDeprecated, self).__init__(**kwargs)
super().__init__(**kwargs)
4 changes: 2 additions & 2 deletions knack/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def __init__(self, cli_ctx, object_type='', target=None, tag_func=None, message_
def _default_get_message(self):
return status_tag_messages['experimental'].format("This " + self.object_type)

super(ExperimentalItem, self).__init__(
super().__init__(
cli_ctx=cli_ctx,
object_type=object_type,
target=target,
Expand All @@ -73,4 +73,4 @@ def get_implicit_experimental_message(self):
'tag_func': lambda _: '',
'message_func': get_implicit_experimental_message
})
super(ImplicitExperimentalItem, self).__init__(**kwargs)
super().__init__(**kwargs)
12 changes: 6 additions & 6 deletions knack/help.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def _normalize_text(s):
def __init__(self, **kwargs):
self._short_summary = ''
self._long_summary = ''
super(HelpObject, self).__init__(**kwargs)
super().__init__(**kwargs)

@property
def short_summary(self):
Expand Down Expand Up @@ -122,7 +122,7 @@ def _load_help_file_from_string(text):
return text

def __init__(self, help_ctx, delimiters): # pylint: disable=too-many-statements
super(HelpFile, self).__init__()
super().__init__()
self.help_ctx = help_ctx
self.delimiters = delimiters
self.name = delimiters.split()[-1] if delimiters else delimiters
Expand Down Expand Up @@ -241,7 +241,7 @@ class GroupHelpFile(HelpFile):

def __init__(self, help_ctx, delimiters, parser):

super(GroupHelpFile, self).__init__(help_ctx, delimiters)
super().__init__(help_ctx, delimiters)
self.type = 'group'

self.children = []
Expand All @@ -265,7 +265,7 @@ class CommandHelpFile(HelpFile):

def __init__(self, help_ctx, delimiters, parser):

super(CommandHelpFile, self).__init__(help_ctx, delimiters)
super().__init__(help_ctx, delimiters)
self.type = 'command'

self.parameters = []
Expand Down Expand Up @@ -324,7 +324,7 @@ def _add_parameter_help(self, param):
self.parameters.append(HelpParameter(**param_kwargs))

def _load_from_data(self, data):
super(CommandHelpFile, self)._load_from_data(data)
super()._load_from_data(data)

if isinstance(data, str) or not self.parameters or not data.get('parameters'):
return
Expand All @@ -344,7 +344,7 @@ class HelpParameter(HelpObject): # pylint: disable=too-many-instance-attributes

def __init__(self, name_source, description, required, choices=None, default=None, group_name=None,
deprecate_info=None, preview_info=None, experimental_info=None, default_value_source=None):
super(HelpParameter, self).__init__()
super().__init__()
self.name_source = name_source
self.name = ' '.join(sorted(name_source))
self.required = required
Expand Down
4 changes: 2 additions & 2 deletions knack/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ def format_table(obj):
should_sort_keys = not obj.is_query_active and not obj.table_transformer
to = _TableOutput(should_sort_keys)
return to.dump(result_list)
except:
except Exception as ex:
logger.debug(traceback.format_exc())
raise CLIError("Table output unavailable. "
"Use the --query option to specify an appropriate query. "
"Use --debug for more info.")
"Use --debug for more info.") from ex


def format_tsv(obj):
Expand Down
6 changes: 3 additions & 3 deletions knack/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def __init__(self, cli_ctx=None, cli_help=None, **kwargs):
# or description for a command. We better stash it away before handing it off for
# "normal" argparse handling...
self._description = kwargs.pop('description', None)
super(CLICommandParser, self).__init__(**kwargs)
super().__init__(**kwargs)

def load_command_table(self, command_loader):
""" Process the command table and load it into the parser
Expand Down Expand Up @@ -221,7 +221,7 @@ def _get_subparser(self, path, group_table=None):
return parent_subparser

def validation_error(self, message):
return super(CLICommandParser, self).error(message)
return super().error(message)

def is_group(self):
""" Determine if this parser instance represents a group
Expand Down Expand Up @@ -257,7 +257,7 @@ def parse_args(self, args=None, namespace=None):
by ArgumentParser.parse_args as usual
"""
self._expand_prefixed_files(args)
return super(CLICommandParser, self).parse_args(args)
return super().parse_args(args)

def _check_value(self, action, value):
# Override to customize the error message when a argument is not among the available choices
Expand Down
4 changes: 2 additions & 2 deletions knack/preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def __init__(self, cli_ctx, object_type='', target=None, tag_func=None, message_
def _default_get_message(self):
return status_tag_messages['preview'].format("This " + self.object_type)

super(PreviewItem, self).__init__(
super().__init__(
cli_ctx=cli_ctx,
object_type=object_type,
target=target,
Expand All @@ -73,4 +73,4 @@ def get_implicit_preview_message(self):
'tag_func': lambda _: '',
'message_func': get_implicit_preview_message
})
super(ImplicitPreviewItem, self).__init__(**kwargs)
super().__init__(**kwargs)
4 changes: 2 additions & 2 deletions knack/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ def jmespath_type(raw_query):
from jmespath import compile as compile_jmespath
try:
return compile_jmespath(raw_query)
except KeyError:
except KeyError as ex:
# Raise a ValueError which argparse can handle
raise ValueError
raise ValueError from ex

@staticmethod
def on_global_arguments(_, **kwargs):
Expand Down
8 changes: 4 additions & 4 deletions knack/testsdk/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

class IntegrationTestBase(unittest.TestCase):
def __init__(self, cli, method_name):
super(IntegrationTestBase, self).__init__(method_name)
super().__init__(method_name)
self.cli = cli
self.diagnose = os.environ.get(ENV_TEST_DIAGNOSE, None) == 'True'

Expand Down Expand Up @@ -82,7 +82,7 @@ class LiveTest(IntegrationTestBase):
class ScenarioTest(IntegrationTestBase): # pylint: disable=too-many-instance-attributes

def __init__(self, cli, method_name, filter_headers=None):
super(ScenarioTest, self).__init__(cli, method_name)
super().__init__(cli, method_name)
self.name_replacer = GeneralNameReplacer()
self.recording_processors = [LargeRequestBodyProcessor(),
LargeResponseBodyProcessor(),
Expand Down Expand Up @@ -113,7 +113,7 @@ def __init__(self, cli, method_name, filter_headers=None):
self.original_env = os.environ.copy()

def setUp(self):
super(ScenarioTest, self).setUp()
super().setUp()

# set up cassette
cm = self.vcr.use_cassette(self.recording_file)
Expand Down Expand Up @@ -251,7 +251,7 @@ def _in_process_execute(self, command):
self.exit_code = self.cli.invoke(shlex.split(command), out_file=out_buffer) or 0
self.output = out_buffer.getvalue()
except vcr.errors.CannotOverwriteExistingCassetteException as ex:
raise AssertionError(ex)
raise AssertionError(ex) from ex
except CliExecutionError as ex:
if ex.exception:
raise ex.exception
Expand Down
12 changes: 6 additions & 6 deletions knack/testsdk/checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ def __call__(self, execution_result):
try:
data = execution_result.output.strip()
assert not data or data in none_strings
except AssertionError:
except AssertionError as ex:
raise AssertionError("Actual value '{}' != Expected value falsy (None, '', []) or "
"string in {}".format(data, none_strings))
"string in {}".format(data, none_strings)) from ex


class StringCheck(object): # pylint: disable=too-few-public-methods
Expand All @@ -76,9 +76,9 @@ def __call__(self, execution_result):
try:
result = execution_result.output.strip().strip('"')
assert result == self.expected_result
except AssertionError:
except AssertionError as ex:
raise AssertionError(
"Actual value '{}' != Expected value {}".format(result, self.expected_result))
"Actual value '{}' != Expected value {}".format(result, self.expected_result)) from ex


class StringContainCheck(object): # pylint: disable=too-few-public-methods
Expand All @@ -89,7 +89,7 @@ def __call__(self, execution_result):
try:
result = execution_result.output.strip('"')
assert self.expected_result in result
except AssertionError:
except AssertionError as ex:
raise AssertionError(
"Actual value '{}' doesn't contain Expected value {}".format(result,
self.expected_result))
self.expected_result)) from ex
7 changes: 3 additions & 4 deletions knack/testsdk/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,18 @@
class CliTestError(Exception):
def __init__(self, error_message):
message = 'An error caused by the CLI test harness failed the test: {}'
super(CliTestError, self).__init__(message.format(error_message))
super().__init__(message.format(error_message))


class CliExecutionError(Exception):
def __init__(self, exception):
self.exception = exception
message = 'The CLI throws exception {} during execution and fails the command.'
super(CliExecutionError, self).__init__(message.format(exception.__class__.__name__,
exception))
super().__init__(message.format(exception.__class__.__name__, exception))


class JMESPathCheckAssertionError(AssertionError):
def __init__(self, query, expected, actual, json_data):
message = "Query '{}' doesn't yield expected value '{}', instead the actual value " \
"is '{}'. Data: \n{}\n".format(query, expected, actual, json_data)
super(JMESPathCheckAssertionError, self).__init__(message)
super().__init__(message)
3 changes: 1 addition & 2 deletions knack/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ class CtxTypeError(TypeError):

def __init__(self, obj):
from .cli import CLI
super(CtxTypeError, self).__init__('expected instance of {} got {}'.format(CLI.__name__,
obj.__class__.__name__))
super().__init__('expected instance of {} got {}'.format(CLI.__name__, obj.__class__.__name__))


class ColorizedString(object):
Expand Down
4 changes: 2 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
argcomplete==1.11.1
colorama==0.4.3
flake8==3.7.9
flake8==3.8.4
jmespath==0.9.5
mock==4.0.1
pylint==2.5.3
pylint==2.6.0
Pygments==2.5.2
PyYAML==5.3.1
six==1.14.0
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cli_scenarios.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def test_should_enable_color(self, stdout_isatty_mock, stderr_isatty_mock):
def test_init_log(self):
class MyCLI(CLI):
def __init__(self, **kwargs):
super(MyCLI, self).__init__(**kwargs)
super().__init__(**kwargs)
self.init_debug_log.append("init debug log: 6aa19a11")
self.init_info_log.append("init info log: b0746f58")
cli = MyCLI()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_command_with_configured_defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def _set_up_command_table(self, required):
class TestCommandsLoader(CLICommandsLoader):

def load_command_table(self, args):
super(TestCommandsLoader, self).load_command_table(args)
super().load_command_table(args)
with CommandGroup(self, 'foo', '{}#{{}}'.format(__name__)) as g:
g.command('list', 'list_foo')
return self.command_table
Expand All @@ -50,7 +50,7 @@ def load_arguments(self, command):
with ArgumentsContext(self, 'foo') as c:
c.argument('my_param', options_list='--my-param',
configured_default='param', required=required)
super(TestCommandsLoader, self).load_arguments(command)
super().load_arguments(command)
self.cli_ctx = DummyCLI(commands_loader_cls=TestCommandsLoader)

@mock.patch.dict(os.environ, {'CLI_DEFAULTS_PARAM': 'myVal'})
Expand Down
Loading