Align python-octaviaclient to octavia coding style

When removing the "mock" module from python-octaviaclient, it was
clear that python-octaviaclient did not follow the same coding
style guides as the main octavia project.

This patch corrects that by updating the code style checks in
python-octaviaclient to those of the octavia repository and
corrects any style issues found by the new tests.

Change-Id: I505b3e139217ff7d4952880dec304eb5667ae310
This commit is contained in:
Michael Johnson 2020-03-13 10:14:04 -07:00
parent 943e0d4aa6
commit 1222d3510c
33 changed files with 271 additions and 71 deletions

99
.pylintrc Normal file
View File

@ -0,0 +1,99 @@
# The format of this file isn't really documented; just use --generate-rcfile
[MASTER]
# Add <file or directory> to the black list. It should be a base name, not a
# path. You may set this option multiple times.
ignore=.git,tests
[MESSAGES CONTROL]
# NOTE: The options which do not need to be suppressed can be removed.
disable=
# "F" Fatal errors that prevent further processing
# "I" Informational noise
c-extension-no-member,
locally-disabled,
# "E" Error for important programming issues (likely bugs)
import-error,
not-callable,
no-member,
# "W" Warnings for stylistic problems or minor programming issues
abstract-method,
anomalous-backslash-in-string,
arguments-differ,
attribute-defined-outside-init,
bad-builtin,
broad-except,
fixme,
global-statement,
no-init,
pointless-string-statement,
protected-access,
redefined-builtin,
redefined-outer-name,
signature-differs,
unidiomatic-typecheck,
unused-argument,
unused-variable,
useless-super-delegation,
# "C" Coding convention violations
bad-continuation,
invalid-name,
line-too-long,
missing-docstring,
# "R" Refactor recommendations
duplicate-code,
interface-not-implemented,
no-self-use,
too-few-public-methods,
too-many-ancestors,
too-many-arguments,
too-many-branches,
too-many-instance-attributes,
too-many-lines,
too-many-locals,
too-many-public-methods,
too-many-return-statements,
too-many-statements,
multiple-statements,
duplicate-except,
keyword-arg-before-vararg,
useless-object-inheritance
[BASIC]
# Variable names can be 1 to 31 characters long, with lowercase and underscores
variable-rgx=[a-z_][a-z0-9_]{0,30}$
# Argument names can be 2 to 31 characters long, with lowercase and underscores
argument-rgx=[a-z_][a-z0-9_]{1,30}$
# Method names should be at least 3 characters long
# and be lowercased with underscores
method-rgx=([a-z_][a-z0-9_]{2,}|setUp|tearDown)$
# Module names matching
module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
# Don't require docstrings on tests.
no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$
[FORMAT]
# Maximum number of characters on a single line.
max-line-length=79
[VARIABLES]
# List of additional names supposed to be defined in builtins. Remember that
# you should avoid to define new builtins when possible.
additional-builtins=
[CLASSES]
[IMPORTS]
# Deprecated modules which should not be used, separated by a comma
deprecated-modules=
[TYPECHECK]
# List of module names for which member attributes should not be checked
ignored-modules=six.moves,_MovedItems
[REPORTS]
# Tells whether to display a full report or only the messages
reports=no

View File

@ -1,16 +1,19 @@
asn1crypto==0.23.0
Babel==2.3.4
bandit==1.4.0
cffi==1.7.0
cliff==2.8.0
coverage==4.0
cryptography==2.1
decorator==3.4.0
deprecation==1.0
doc8==0.6.0
dogpile.cache==0.6.2
extras==1.0.0
fixtures==3.0.0
flake8==2.5.5
hacking==0.12.0
flake8-import-order==0.12
hacking==1.1.0
idna==2.6
jmespath==0.9.0
jsonpatch==1.16
@ -34,6 +37,7 @@ pep8==1.5.7
positional==1.2.1
pycparser==2.18
pyflakes==0.8.1
pylint==1.9.2
pyOpenSSL==17.1.0
pyperclip==1.5.27
python-cinderclient==3.3.0
@ -45,7 +49,7 @@ python-novaclient==9.1.0
python-openstackclient==3.12.0
python-subunit==1.0.0
requests==2.14.2
requests-mock==1.1.0
requests-mock==1.2.0
rfc3986==0.3.1
stestr==2.0.0
testscenarios==0.4

View File

@ -18,6 +18,7 @@ class OctaviaClientException(Exception):
def __init__(self, code, message=None, request_id=None):
self.code = code
self.message = message or self.__class__.message
super(OctaviaClientException, self).__init__(self.message)
self.request_id = request_id
def __str__(self):

View File

@ -10,7 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
#
"""Octavia API Library"""
import functools
@ -46,8 +45,8 @@ def correct_return_codes(func):
_status_dict.get(code, message))
except Exception:
message = _status_dict.get(code, message)
elif (isinstance(e, osc_exc.ClientException)
and e.code != e.http_status):
elif (isinstance(e, osc_exc.ClientException) and
e.code != e.http_status):
# cover https://review.opendev.org/675328 case
code = e.http_status
message = e.code

View File

@ -41,7 +41,7 @@ _log_translation_hint = re.compile(
assert_trueinst_re = re.compile(
r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
"(\w|\.|\'|\"|\[|\])+\)\)")
r"(\w|\.|\'|\"|\[|\])+\)\)")
assert_equal_in_end_with_true_or_false_re = re.compile(
r"assertEqual\((\w|[][.'\"])+ in (\w|[][.'\", ])+, (True|False)\)")
assert_equal_in_start_with_true_or_false_re = re.compile(

View File

@ -9,14 +9,13 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""OpenStackClient plugin for Load Balancer service."""
import logging
from octaviaclient.api.v2 import octavia
from osc_lib import utils
from octaviaclient.api.v2 import octavia
LOG = logging.getLogger(__name__)
DEFAULT_LOADBALANCER_API_VERSION = '2.0'

View File

@ -80,8 +80,7 @@ class ListAmphora(lister.Lister):
(utils.get_dict_properties(
amp,
columns,
formatters=formatters,
) for amp in data['amphorae']),
formatters=formatters) for amp in data['amphorae']),
)

View File

@ -237,7 +237,7 @@ class UnsetAvailabilityzone(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
availabilityzone_id = v2_utils.get_resource_id(

View File

@ -247,7 +247,7 @@ class UnsetFlavor(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
flavor_id = v2_utils.get_resource_id(

View File

@ -441,7 +441,7 @@ class UnsetHealthMonitor(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
hm_id = v2_utils.get_resource_id(

View File

@ -382,7 +382,7 @@ class UnsetL7Policy(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
policy_id = v2_utils.get_resource_id(

View File

@ -370,7 +370,7 @@ class UnsetL7Rule(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
policy_id = v2_utils.get_resource_id(

View File

@ -589,7 +589,7 @@ class UnsetListener(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
listener_id = v2_utils.get_resource_id(

View File

@ -488,7 +488,7 @@ class UnsetLoadBalancer(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
lb_id = v2_utils.get_resource_id(

View File

@ -418,7 +418,7 @@ class UnsetMember(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
pool_id = v2_utils.get_resource_id(

View File

@ -421,7 +421,7 @@ class UnsetPool(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
pool_id = v2_utils.get_resource_id(

View File

@ -236,7 +236,7 @@ class UnsetQuota(command.Command):
def take_action(self, parsed_args):
unset_args = v2_utils.get_unsets(parsed_args)
if not len(unset_args):
if not unset_args:
return
project_id = v2_utils.get_resource_id(

View File

@ -93,26 +93,23 @@ def get_resource_id(resource, resource_name, name):
name
).id
return project_id
else:
return 'non-uuid'
elif resource_name == 'members':
if resource_name == 'members':
names = [re for re in resource(name['pool_id'])['members']
if re.get('id') == name['member_id']
or re.get('name') == name['member_id']]
if re.get('id') == name['member_id'] or
re.get('name') == name['member_id']]
name = name['member_id']
if len(names) > 1:
msg = ("{0} {1} found with name or ID of {2}. Please try "
"again with UUID".format(len(names), resource_name,
name))
raise osc_exc.CommandError(msg)
else:
return names[0].get('id')
elif resource_name == 'l7rules':
if resource_name == 'l7rules':
names = [re for re in resource(name['l7policy_id'])['rules']
if re.get('id') == name['l7rule_id']]
name = name['l7rule_id']
return names[0].get('id')
else:
names = [re for re in resource()[resource_name]
if re.get('name') == name or re.get('id') == name]
if len(names) > 1:
@ -120,7 +117,6 @@ def get_resource_id(resource, resource_name, name):
"again with UUID".format(len(names), resource_name,
name))
raise osc_exc.CommandError(msg)
else:
return names[0].get(primary_key)
except IndexError:
@ -581,7 +577,6 @@ def format_list_flat(data):
def format_hash(data):
if data:
return '\n'.join('{}={}'.format(k, v) for k, v in data.items())
else:
return None

View File

@ -44,7 +44,7 @@ def execute(cmd, fail_ok=False, merge_stderr=False):
class TestCase(utils.TestCase):
delimiter_line = re.compile('^\+\-[\+\-]+\-\+$')
delimiter_line = re.compile(r'^\+\-[\+\-]+\-\+$')
@classmethod
def openstack(cls, cmd, fail_ok=False):

View File

@ -25,8 +25,7 @@ class TestValidations(utils.TestCommand):
def test_check_l7policy_attrs(self):
attrs_dict = {
"action": "redirect_to_pool".upper(),
"redirect_pool_id": "id",
}
"redirect_pool_id": "id"}
try:
validate.check_l7policy_attrs(attrs_dict)
except exceptions.CommandError as e:
@ -38,8 +37,7 @@ class TestValidations(utils.TestCommand):
attrs_dict = {
"action": "redirect_to_url".upper(),
"redirect_url": "url",
}
"redirect_url": "url"}
try:
validate.check_l7policy_attrs(attrs_dict)
except exceptions.CommandError as e:

View File

@ -233,4 +233,4 @@ class HackingTestCase(base.BaseTestCase):
return check_fns
def test_factory(self):
self.assertTrue(len(self._get_factory_checks(checks.factory)) > 0)
self.assertGreater(len(self._get_factory_checks(checks.factory)), 0)

View File

@ -2,9 +2,13 @@
# of appearance. Changing the order has an impact on the overall integration
# process, which may cause wedges in the gate later.
hacking!=0.13.0,<0.14,>=0.12.0 # Apache-2.0
requests-mock>=1.1.0 # Apache-2.0
hacking>=1.1.0 # Apache-2.0
requests-mock>=1.2.0 # Apache-2.0
coverage!=4.4,>=4.0 # Apache-2.0
doc8>=0.6.0 # Apache-2.0
bandit!=1.6.0,>=1.1.0 # Apache-2.0
flake8-import-order==0.12 # LGPLv3
pylint>=2.2.0 # GPLv2
python-subunit>=1.0.0 # Apache-2.0/BSD
oslotest>=3.2.0 # Apache-2.0
stestr>=2.0.0 # Apache-2.0

66
tools/coding-checks.sh Executable file
View File

@ -0,0 +1,66 @@
#!/bin/sh
# This script is copied from neutron and adapted for octavia.
set -eu
usage () {
echo "Usage: $0 [OPTION]..."
echo "Run octavia's coding check(s)"
echo ""
echo " -Y, --pylint [<basecommit>] Run pylint check on the entire octavia module or just files changed in basecommit (e.g. HEAD~1)"
echo " -h, --help Print this usage message"
echo
exit 0
}
join_args() {
if [ -z "$scriptargs" ]; then
scriptargs="$opt"
else
scriptargs="$scriptargs $opt"
fi
}
process_options () {
i=1
while [ $i -le $# ]; do
eval opt=\$$i
case $opt in
-h|--help) usage;;
-Y|--pylint) pylint=1;;
*) join_args;;
esac
i=$((i+1))
done
}
run_pylint () {
local target="${scriptargs:-all}"
if [ "$target" = "all" ]; then
files="octaviaclient"
else
case "$target" in
*HEAD~[0-9]*) files=$(git diff --diff-filter=AM --name-only $target -- "*.py");;
*) echo "$target is an unrecognized basecommit"; exit 1;;
esac
fi
echo "Running pylint..."
echo "You can speed this up by running it on 'HEAD~[0-9]' (e.g. HEAD~1, this change only)..."
if [ -n "${files}" ]; then
pylint -j 0 --max-nested-blocks 7 --rcfile=.pylintrc --output-format=colorized ${files}
else
echo "No python changes in this commit, pylint check not required."
exit 0
fi
}
scriptargs=
pylint=1
process_options $@
if [ $pylint -eq 1 ]; then
run_pylint
exit 0
fi

40
tox.ini
View File

@ -2,6 +2,7 @@
minversion = 2.5.0
envlist = py37,pep8
skipsdist = True
ignore_basepython_conflict = True
[testenv]
basepython = python3
@ -22,7 +23,17 @@ commands =
stestr slowest
[testenv:pep8]
commands = flake8 {posargs}
commands = flake8
# RST linter
doc8 --ignore-path doc/source/contributor/modules doc/source \
octaviaclient HACKING.rst README.rst
# Run security linter
{[testenv:bandit]commands}
{toxinidir}/tools/coding-checks.sh --pylint {posargs}
whitelist_externals =
sh
find
bash
[testenv:venv]
commands = {posargs}
@ -77,10 +88,32 @@ commands =
oslo_debug_helper -t octaviaclient/tests {posargs}
[flake8]
ignore =
# [H104]: Empty file with only comments
# [W504]: Line break after binary operator
ignore = H104,W504
show-source = true
builtins = _
exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build
import-order-style = pep8
# [H106]: Don't put vim configuration in source files
# [H203]: Use assertIs(Not)None to check for None
# [H204]: Use assert(Not)Equal to check for equality
# [H205]: Use assert(Greater|Less)(Equal) for comparison
# [H904]: Delay string interpolations at logging calls
enable-extensions=H106,H203,H204,H205,H904
[testenv:bashate]
envdir = {toxworkdir}/shared
commands = bash -c "find {toxinidir} \
-not \( -type d -name .tox\* -prune \) \
-not \( -type d -name .venv\* -prune \) \
-type f \
-name \*.sh \
# [E005]: File does not begin with #! or have a .sh prefix
# [E006]: Check for lines longer than 79 columns
# [E042]: Local declaration hides errors
# [E043]: Arithmetic compound has inconsistent return semantics
-print0 | xargs -0 bashate -v -iE006 -eE005,E042,E043"
[hacking]
local-check-factory = octaviaclient.hacking.checks.factory
@ -97,3 +130,6 @@ deps =
whitelist_externals = sh
commands =
sh -c '{envdir}/src/openstack-requirements/playbooks/files/project-requirements-change.py --req {envdir}/src/openstack-requirements --local {toxinidir} master'
[testenv:bandit]
commands = bandit -r octaviaclient -ll -ii -x tests