Skip to content
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
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ arch:
sudo: false
language: python
python:
- '3.7'
- '3.8'
- '3.9'
- '3.10'
- '3.11'
install: pip install tox-travis
script: tox
9 changes: 4 additions & 5 deletions azure-pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ jobs:
name: 'pool-ubuntu-2004'
strategy:
matrix:
Python37:
python.version: '3.7'
tox_env: 'py37'
Python38:
python.version: '3.8'
tox_env: 'py38'
Expand All @@ -32,6 +29,9 @@ jobs:
Python310:
python.version: '3.10'
tox_env: 'py310'
Python311:
python.version: '3.11'
tox_env: 'py311'
steps:
- task: UsePythonVersion@0
displayName: 'Use Python $(python.version)'
Expand All @@ -53,9 +53,8 @@ jobs:
name: 'pool-ubuntu-2004'
steps:
- task: UsePythonVersion@0
displayName: Use Python 3.10
Copy link
Member Author

@jiasli jiasli Jul 21, 2023

Choose a reason for hiding this comment

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

Using the default displayName is sufficient - it is the same as what we are specifying here.

image

inputs:
versionSpec: 3.10
versionSpec: 3.11
- bash: |
set -ev

Expand Down
6 changes: 3 additions & 3 deletions knack/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,15 @@ def __init__(self,
def _should_show_version(args):
return args and (args[0] == '--version' or args[0] == '-v')

def get_cli_version(self): # pylint: disable=no-self-use
def get_cli_version(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

The removal of # pylint: disable=no-self-use is explained in Azure/azure-cli-dev-tools#359 (comment)

""" Get the CLI Version. Override this to define how to get the CLI version

:return: The CLI version
:rtype: str
"""
return ''

def get_runtime_version(self): # pylint: disable=no-self-use
def get_runtime_version(self):
""" Get the runtime information.

:return: Runtime information
Expand Down Expand Up @@ -169,7 +169,7 @@ def raise_event(self, event_name, **kwargs):
for func in handlers:
func(self, **kwargs)

def exception_handler(self, ex): # pylint: disable=no-self-use
def exception_handler(self, ex):
""" The default exception handler """
if isinstance(ex, CLIError):
logger.error(ex)
Expand Down
2 changes: 1 addition & 1 deletion knack/completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def __init__(self, cli_ctx=None):
self.cli_ctx = cli_ctx
self.cli_ctx.data['completer_active'] = ARGCOMPLETE_ENV_NAME in os.environ

def get_completion_args(self, is_completion=False, comp_line=None): # pylint: disable=no-self-use
def get_completion_args(self, is_completion=False, comp_line=None):
""" Get the args that will be used to tab completion if completion is active. """
is_completion = is_completion or os.environ.get(ARGCOMPLETE_ENV_NAME)
comp_line = comp_line or os.environ.get('COMP_LINE')
Expand Down
6 changes: 2 additions & 4 deletions knack/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,10 @@ def _default_get_message(self):
message_func=message_func or _default_get_message
)

# pylint: disable=no-self-use
def _version_less_than_or_equal_to(self, v1, v2):
""" Returns true if v1 <= v2. """
# pylint: disable=no-name-in-module, import-error
from distutils.version import LooseVersion
return LooseVersion(v1) <= LooseVersion(v2)
from packaging.version import parse
return parse(v1) <= parse(v2)
Comment on lines +99 to +100
Copy link
Member Author

Choose a reason for hiding this comment

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

Similar changes were previously made in Azure/azure-cli#17667.

PEP 632 mentions:

distutils.version — use the packaging package


def expired(self):
if self.expiration:
Expand Down
34 changes: 6 additions & 28 deletions knack/introspection.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,45 +67,23 @@ def extract_args_from_signature(operation, excluded_params=None):
""" Extracts basic argument data from an operation's signature and docstring
excluded_params: List of params to ignore and not extract. By default we ignore ['self', 'kwargs'].
"""
args = []
try:
# only supported in python3 - falling back to argspec if not available
sig = inspect.signature(operation)
args = sig.parameters
except AttributeError:
sig = inspect.getargspec(operation) # pylint: disable=deprecated-method, useless-suppression
args = sig.args
sig = inspect.signature(operation)
args = sig.parameters
Comment on lines +70 to +71
Copy link
Member Author

Choose a reason for hiding this comment

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

inspect.getargspec is removed in Python 3.11 and replaced by inspect.getfullargspec. However, inspect.getfullargspec is retained only to maintain backward compatibility. The recommendation is to use inspect.signature:

Note that signature() and Signature Object provide the recommended API for callable introspection, and support additional behaviours (like positional-only arguments) that are sometimes encountered in extension module APIs. This function is retained primarily for use in code that needs to maintain compatibility with the Python 2 inspect module API.
-- https://docs.python.org/3.11/library/inspect.html#inspect.getfullargspec

Given we have dropped Python 2 long ago (#233), it's time to drop the usage of inspect.getargspec too.


arg_docstring_help = option_descriptions(operation)
excluded_params = excluded_params or ['self', 'kwargs']

for arg_name in [a for a in args if a not in excluded_params]:
try:
# this works in python3
default = args[arg_name].default
required = default == inspect.Parameter.empty # pylint: disable=no-member, useless-suppression
except TypeError:
arg_defaults = (dict(zip(sig.args[-len(sig.defaults):], sig.defaults))
if sig.defaults
else {})
default = arg_defaults.get(arg_name)
required = arg_name not in arg_defaults

default = args[arg_name].default
required = default == inspect.Parameter.empty
action = 'store_' + str(not default).lower() if isinstance(default, bool) else None

try:
default = (default
if default != inspect._empty # pylint: disable=protected-access
else None)
except AttributeError:
pass

command_argument_default = default if default != inspect.Parameter.empty else None
Copy link
Member Author

Choose a reason for hiding this comment

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

The original code reuses the variable name default to denote different things, which is a bad practice. We rename it to command_argument_default to distinguish command argument default from function argument default.

options_list = ['--' + arg_name.replace('_', '-')]
help_str = arg_docstring_help.get(arg_name)

yield (arg_name, CLICommandArgument(arg_name,
options_list=options_list,
required=required,
default=default,
default=command_argument_default,
help=help_str,
action=action))
6 changes: 3 additions & 3 deletions knack/invocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def __init__(self,
prog=self.cli_ctx.name, parents=[self._global_parser])
self.commands_loader = commands_loader_cls(cli_ctx=self.cli_ctx)

def _filter_params(self, args): # pylint: disable=no-self-use
def _filter_params(self, args):
# Consider - we are using any args that start with an underscore (_) as 'private'
# arguments and remove them from the arguments that we pass to the actual function.
params = {key: value
Expand Down Expand Up @@ -88,15 +88,15 @@ def _find_args(args):

return ' '.join(nouns)

def _validate_cmd_level(self, ns, cmd_validator): # pylint: disable=no-self-use
def _validate_cmd_level(self, ns, cmd_validator):
if cmd_validator:
cmd_validator(ns)
try:
delattr(ns, '_command_validator')
except AttributeError:
pass

def _validate_arg_level(self, ns, **_): # pylint: disable=no-self-use
def _validate_arg_level(self, ns, **_):
for validator in getattr(ns, '_argument_validators', []):
validator(ns)
try:
Expand Down
2 changes: 1 addition & 1 deletion knack/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def _is_file_log_enabled(cli_ctx):

@staticmethod
def _get_log_dir(cli_ctx):
default_dir = (os.path.join(cli_ctx.config.config_dir, 'logs'))
default_dir = os.path.join(cli_ctx.config.config_dir, 'logs')
return os.path.expanduser(cli_ctx.config.get('logging', 'log_dir', fallback=default_dir))

def _get_console_log_levels(self):
Expand Down
4 changes: 2 additions & 2 deletions knack/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def __init__(self, cli_ctx=None):
self.cli_ctx.register_event(EVENT_PARSER_GLOBAL_CREATE, OutputProducer.on_global_arguments)
self.cli_ctx.register_event(EVENT_INVOKER_POST_PARSE_ARGS, OutputProducer.handle_output_argument)

def out(self, obj, formatter=None, out_file=None): # pylint: disable=no-self-use
def out(self, obj, formatter=None, out_file=None):
""" Produces the output using the command result.
The method does not return a result as the output is written straight to the output file.

Expand Down Expand Up @@ -157,7 +157,7 @@ def out(self, obj, formatter=None, out_file=None): # pylint: disable=no-self-us
print(output.encode('ascii', 'ignore').decode('utf-8', 'ignore'),
file=out_file, end='')

def get_formatter(self, format_type): # pylint: disable=no-self-use
def get_formatter(self, format_type):
# remove color if stdout is not a tty
if not self.cli_ctx.enable_color and format_type == 'jsonc':
return OutputProducer._FORMAT_DICT['json']
Expand Down
4 changes: 2 additions & 2 deletions knack/testsdk/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def __init__(self, cli, method_name):
def cmd(self, command, checks=None, expect_failure=False):
return ExecutionResult(self.cli, command, expect_failure=expect_failure).assert_with_checks(checks)

def create_random_name(self, prefix, length): # pylint: disable=no-self-use
def create_random_name(self, prefix, length):
return create_random_name(prefix=prefix, length=length)

def create_temp_file(self, size_kb, full_random=False):
Expand Down Expand Up @@ -117,7 +117,7 @@ def setUp(self):

# set up cassette
cm = self.vcr.use_cassette(self.recording_file)
self.cassette = cm.__enter__()
self.cassette = cm.__enter__() # pylint: disable=unnecessary-dunder-call
Copy link
Member Author

@jiasli jiasli Jul 21, 2023

Choose a reason for hiding this comment

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

cm.__exit__ is not called immediately in this method to release self.cassette, but registered in self.addCleanup, so this dunder call is not unnecessary.

self.addCleanup(cm.__exit__)

if not self.in_recording:
Expand Down
2 changes: 1 addition & 1 deletion knack/testsdk/patches.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ def _mock_in_unit_test(unit_test, target, replacement):
if not isinstance(unit_test, unittest.TestCase):
raise CliTestError('The patch can be only used in unit test')
mp = mock.patch(target, replacement)
mp.__enter__()
mp.__enter__() # pylint: disable=unnecessary-dunder-call
unit_test.addCleanup(mp.__exit__)
4 changes: 2 additions & 2 deletions knack/testsdk/recording_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@


class RecordingProcessor(object):
def process_request(self, request): # pylint: disable=no-self-use
def process_request(self, request):
return request

def process_response(self, response): # pylint: disable=no-self-use
def process_response(self, response):
return response

@classmethod
Expand Down
1 change: 0 additions & 1 deletion knack/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ def __init__(self, cli_ctx, object_type, target, tag_func, message_func, color,
self._get_tag = tag_func
self._get_message = message_func

# pylint: disable=no-self-use
def hidden(self):
return False

Expand Down
17 changes: 9 additions & 8 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
argcomplete==1.12.2
flake8==4.0.1
jmespath==0.10.0
Pygments==2.8.1
pylint==2.11.1
pytest==6.2.5
argcomplete==3.1.1
flake8==6.0.0
jmespath==1.0.1
packaging==23.1
Pygments==2.15.1
pylint==2.17.4
pytest==7.4.0
PyYAML
tabulate==0.8.9
vcrpy==4.1.1
tabulate==0.9.0
vcrpy==5.0.0
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
DEPENDENCIES = [
'argcomplete',
'jmespath',
'packaging',
'pygments',
'pyyaml',
'tabulate'
Expand All @@ -37,10 +38,10 @@
'Intended Audience :: System Administrators',
'Programming Language :: Python',
'Programming Language :: Python :: 3',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
'License :: OSI Approved :: MIT License',
],
packages=['knack', 'knack.testsdk'],
Expand Down
4 changes: 2 additions & 2 deletions tests/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ class TestQuery(unittest.TestCase):
(We are not testing JMESPath itself here)
'''

def test_query_valid_1(self): # pylint: disable=no-self-use
def test_query_valid_1(self):
query = 'length(@)'
# Should not raise any exception as it is valid
CLIQuery.jmespath_type(query)

def test_query_valid_2(self): # pylint: disable=no-self-use
def test_query_valid_2(self):
query = "[?propertyX.propertyY.propertyZ=='AValue'].[col1,col2]"
# Should not raise any exception as it is valid
CLIQuery.jmespath_type(query)
Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
[tox]
envlist = py36,py37,py38,py39,py310
envlist = py38,py39,py310,py311
[testenv]
deps = -rrequirements.txt
commands=
python ./scripts/license_verify.py
flake8 --statistics --append-config=.flake8 knack
pylint knack --rcfile=.pylintrc -r n -d I0013
pylint knack --rcfile=.pylintrc --reports n --disable I0013
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 arguments' full name for clarity.

It's very likely we need to remove --disable I0013 in the near future, as pylint 3.0.0b1 disables it by default:

This message is disabled by default. To enable it, add file-ignored to the enable option.
-- https://pylint.pycqa.org/en/latest/user_guide/messages/information/file-ignored.html

pytest
python ./examples/test_exapp