Skip to content

Commit

Permalink
Bump to hacking 1.1.0
Browse files Browse the repository at this point in the history
This brings in a couple of new checks which must be addressed, many of
which involve a rather large amount of changes, so these are ignored for
now. A series of follow-up changes will resolved these.

'pycodestyle' is added as a dependency rather than it being pulled in
transitively. This is necessary since we're using it in tests.

Change-Id: I35c654bd39f343417e0a1124263ff31dcd0b05c9
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
  • Loading branch information
stephenfin committed Apr 12, 2019
1 parent e18b79e commit 3e65f77
Show file tree
Hide file tree
Showing 44 changed files with 172 additions and 148 deletions.
2 changes: 1 addition & 1 deletion lower-constraints.txt
Expand Up @@ -101,7 +101,6 @@ paramiko==2.0.0
Paste==2.0.2
PasteDeploy==1.5.0
pbr==2.0.0
pep8==1.5.7
pluggy==0.6.0
ply==3.11
prettytable==0.7.1
Expand All @@ -113,6 +112,7 @@ pyasn1-modules==0.2.1
pycadf==2.7.0
pycparser==2.18
pyflakes==0.8.1
pycodestyle==2.0.0
pyinotify==0.9.6
pyroute2==0.5.4
PyJWT==1.7.0
Expand Down
8 changes: 5 additions & 3 deletions nova/api/openstack/compute/availability_zone.py
Expand Up @@ -88,9 +88,11 @@ def _describe_availability_zones_verbose(self, context, **kwargs):
hosts[host] = {}
for service in host_services[zone + host]:
alive = self.servicegroup_api.service_is_up(service)
hosts[host][service['binary']] = {'available': alive,
'active': True != service['disabled'],
'updated_at': service['updated_at']}
hosts[host][service['binary']] = {
'available': alive,
'active': service['disabled'] is not True,
'updated_at': service['updated_at']
}
result.append({'zoneName': zone,
'zoneState': {'available': True},
"hosts": hosts})
Expand Down
2 changes: 2 additions & 0 deletions nova/api/openstack/compute/tenant_networks.py
Expand Up @@ -190,4 +190,6 @@ def _register_network_quota():
QUOTAS.register_resource(quota.CountableResource('networks',
_network_count,
'quota_networks'))


_register_network_quota()
6 changes: 3 additions & 3 deletions nova/api/openstack/placement/exception.py
Expand Up @@ -67,7 +67,7 @@ class NotFound(_BaseException):


class Exists(_BaseException):
msg_fmt = _("Resource already exists.")
msg_fmt = _("Resource already exists.")


class InvalidInventory(_BaseException):
Expand Down Expand Up @@ -188,15 +188,15 @@ class ProjectNotFound(NotFound):


class ProjectExists(Exists):
msg_fmt = _("The project %(external_id)s already exists.")
msg_fmt = _("The project %(external_id)s already exists.")


class UserNotFound(NotFound):
msg_fmt = _("No such user(s): %(external_id)s.")


class UserExists(Exists):
msg_fmt = _("The user %(external_id)s already exists.")
msg_fmt = _("The user %(external_id)s already exists.")


class ConsumerNotFound(NotFound):
Expand Down
4 changes: 2 additions & 2 deletions nova/api/openstack/urlmap.py
Expand Up @@ -19,13 +19,13 @@
import paste.urlmap
import six

from nova.api.openstack import wsgi

if six.PY2:
import urllib2
else:
from urllib import request as urllib2

from nova.api.openstack import wsgi


LOG = logging.getLogger(__name__)

Expand Down
1 change: 1 addition & 0 deletions nova/api/validation/parameter_types.py
Expand Up @@ -164,6 +164,7 @@ def valid_char(char):
regex += "-" + re.escape(c)
return regex


valid_name_regex_base = '^(?![%s])[%s]*(?<![%s])$'


Expand Down
6 changes: 3 additions & 3 deletions nova/cmd/manage.py
Expand Up @@ -1677,9 +1677,9 @@ def update_cell(self, cell_uuid, name=None, transport_url=None,

if (self._non_unique_transport_url_database_connection_checker(ctxt,
cell_mapping, transport_url, db_connection)):
# We use the return code 3 before 2 to avoid changing the
# semantic meanings of return codes.
return 3
# We use the return code 3 before 2 to avoid changing the
# semantic meanings of return codes.
return 3

if transport_url:
cell_mapping.transport_url = transport_url
Expand Down
10 changes: 5 additions & 5 deletions nova/compute/multi_cell_list.py
Expand Up @@ -413,7 +413,7 @@ def do_query(cctx):

if context.is_cell_failure_sentinel(item._db_record):
if (not CONF.api.list_records_by_skipping_down_cells and
not cell_down_support):
not cell_down_support):
# Value the config
# ``CONF.api.list_records_by_skipping_down_cells`` only if
# cell_down_support is False and generate the exception
Expand All @@ -422,10 +422,10 @@ def do_query(cctx):
# be skipped now to either construct minimal constructs
# later if cell_down_support is True or to simply return
# the skipped results if cell_down_support is False.
raise exception.NovaException(
_('Cell %s is not responding but configuration '
'indicates that we should fail.')
% item.cell_uuid)
raise exception.NovaException(
_('Cell %s is not responding but configuration '
'indicates that we should fail.')
% item.cell_uuid)
LOG.warning('Cell %s is not responding and hence is '
'being omitted from the results',
item.cell_uuid)
Expand Down
1 change: 1 addition & 0 deletions nova/conf/database.py
Expand Up @@ -105,6 +105,7 @@ def get_db_opts():
# texts here if needed.
alt_db_opt.help = db_opt.help + alt_db_opt.help


# NOTE(cdent): See the note above on api_db_group. The same issues
# apply here.

Expand Down
3 changes: 2 additions & 1 deletion nova/context.py
Expand Up @@ -232,7 +232,8 @@ def elevated(self, read_deleted=None):
return context

def can(self, action, target=None, fatal=True):
"""Verifies that the given action is valid on the target in this context.
"""Verifies that the given action is valid on the target in this
context.
:param action: string representing the action to be checked.
:param target: dictionary representing the object of the action
Expand Down
17 changes: 8 additions & 9 deletions nova/hacking/checks.py
Expand Up @@ -17,7 +17,6 @@
import os
import re

import pep8
import six

"""
Expand Down Expand Up @@ -122,7 +121,7 @@ class BaseASTChecker(ast.NodeVisitor):
"""

def __init__(self, tree, filename):
"""This object is created automatically by pep8.
"""This object is created automatically by pycodestyle.
:param tree: an AST tree
:param filename: name of the file being analyzed
Expand All @@ -132,12 +131,12 @@ def __init__(self, tree, filename):
self._errors = []

def run(self):
"""Called automatically by pep8."""
"""Called automatically by pycodestyle."""
self.visit(self._tree)
return self._errors

def add_error(self, node, message=None):
"""Add an error caused by a node to the list of errors for pep8."""
"""Add an error caused by a node to the list of errors."""
message = message or self.CHECK_DESC
error = (node.lineno, node.col_offset, message, self.__class__)
self._errors.append(error)
Expand Down Expand Up @@ -558,10 +557,10 @@ def assert_equal_in(logical_line):
"contents.")


def check_http_not_implemented(logical_line, physical_line, filename):
def check_http_not_implemented(logical_line, physical_line, filename, noqa):
msg = ("N339: HTTPNotImplemented response must be implemented with"
" common raise_feature_not_supported().")
if pep8.noqa(physical_line):
if noqa:
return
if ("nova/api/openstack/compute" not in filename):
return
Expand Down Expand Up @@ -722,7 +721,7 @@ def no_log_warn(logical_line):
yield (0, msg)


def check_context_log(logical_line, physical_line, filename):
def check_context_log(logical_line, physical_line, filename, noqa):
"""check whether context is being passed to the logs
Not correct: LOG.info(_LI("Rebooting instance"), context=context)
Expand All @@ -731,10 +730,10 @@ def check_context_log(logical_line, physical_line, filename):
N353
"""
if "nova/tests" in filename:
if noqa:
return

if pep8.noqa(physical_line):
if "nova/tests" in filename:
return

if log_remove_context.match(logical_line):
Expand Down
1 change: 1 addition & 0 deletions nova/monkey_patch.py
Expand Up @@ -88,6 +88,7 @@ def _monkey_patch():
"importing and not executing nova code.",
', '.join(problems))


# NOTE(mdbooth): This workaround is required to avoid breaking sphinx. See
# separate comment in doc/source/conf.py. It may also be useful for other
# non-nova utilities. Ideally the requirement for this workaround will be
Expand Down
1 change: 1 addition & 0 deletions nova/network/linux_net.py
Expand Up @@ -57,6 +57,7 @@ def get_binary_name():
"""Grab the name of the binary we're running in."""
return os.path.basename(inspect.stack()[-1][1])[:16]


binary_name = get_binary_name()


Expand Down
26 changes: 13 additions & 13 deletions nova/rpc.py
Expand Up @@ -12,19 +12,6 @@
# License for the specific language governing permissions and limitations
# under the License.

__all__ = [
'init',
'cleanup',
'set_defaults',
'add_extra_exmods',
'clear_extra_exmods',
'get_allowed_exmods',
'RequestContextSerializer',
'get_client',
'get_server',
'get_notifier',
]

import functools

from oslo_log import log as logging
Expand All @@ -40,6 +27,19 @@
import nova.exception
from nova.i18n import _

__all__ = [
'init',
'cleanup',
'set_defaults',
'add_extra_exmods',
'clear_extra_exmods',
'get_allowed_exmods',
'RequestContextSerializer',
'get_client',
'get_server',
'get_notifier',
]

profiler = importutils.try_import("osprofiler.profiler")


Expand Down
8 changes: 4 additions & 4 deletions nova/tests/functional/api_sample_tests/test_compare_result.py
Expand Up @@ -469,7 +469,7 @@ def test_template_no_subs_key(self):
response_data = 'bar'

with testtools.ExpectedException(KeyError):
self.ast._compare_result(
expected=template_data,
result=response_data,
result_str="Test")
self.ast._compare_result(
expected=template_data,
result=response_data,
result_str="Test")
2 changes: 1 addition & 1 deletion nova/tests/functional/api_samples_test_base.py
Expand Up @@ -522,7 +522,7 @@ def _do_post(self, url, name=None, subs=None, method='POST', headers=None):
body = self._read_template(name) % self.subs
sample = self._get_sample(name, self.microversion)
if self.generate_samples and not os.path.exists(sample):
self._write_sample(name, body)
self._write_sample(name, body)
return self._get_response(url, method, body, headers=headers)

def _do_put(self, url, name=None, subs=None, headers=None):
Expand Down
6 changes: 3 additions & 3 deletions nova/tests/functional/integrated_helpers.py
Expand Up @@ -254,9 +254,9 @@ def _wait_for_server_parameter(self, admin_api, server, expected_params,
break
retry_count += 1
if retry_count == max_retries:
self.fail('Wait for state change failed, '
'expected_params=%s, server=%s'
% (expected_params, server))
self.fail('Wait for state change failed, '
'expected_params=%s, server=%s' % (
expected_params, server))
time.sleep(0.5)

return server
Expand Down
2 changes: 1 addition & 1 deletion nova/tests/unit/api/openstack/compute/test_cells.py
Expand Up @@ -84,7 +84,7 @@ def insecure_transport_url(url):
cells[1]['transport_url'] = insecure_transport_url(
cells[1]['transport_url'])
for i, cell in enumerate(cells):
cell['capabilities'] = self.fake_capabilities[i]
cell['capabilities'] = self.fake_capabilities[i]
return cells


Expand Down
6 changes: 3 additions & 3 deletions nova/tests/unit/api/openstack/compute/test_flavor_access.py
Expand Up @@ -75,9 +75,9 @@ def fake_get_flavor_by_flavor_id(context, flavorid):

def _has_flavor_access(flavorid, projectid):
for access in ACCESS_LIST:
if access['flavor_id'] == flavorid and \
access['project_id'] == projectid:
return True
if (access['flavor_id'] == flavorid and
access['project_id'] == projectid):
return True
return False


Expand Down
13 changes: 7 additions & 6 deletions nova/tests/unit/api/openstack/compute/test_floating_ips.py
Expand Up @@ -88,12 +88,13 @@ def network_api_disassociate(self, context, instance, floating_address):


def fake_instance_get(context, instance_id):
return objects.Instance(**{
return objects.Instance(**{
"id": 1,
"uuid": uuids.fake,
"name": 'fake',
"user_id": 'fakeuser',
"project_id": '123'})
"project_id": '123'
})


def stub_nw_info(test):
Expand Down Expand Up @@ -759,8 +760,8 @@ def fake_network_api_associate(self, context, instance,
fixed_address=None):
floating_ips = ["10.10.10.10", "10.10.10.11"]
if floating_address not in floating_ips:
raise exception.FloatingIpNotFoundForAddress(
address=floating_address)
raise exception.FloatingIpNotFoundForAddress(
address=floating_address)

self.stubs.Set(network.api.API, "associate_floating_ip",
fake_network_api_associate)
Expand All @@ -775,8 +776,8 @@ def network_api_get_floating_ip_by_address(self, context,
floating_address):
floating_ips = ["10.10.10.10", "10.10.10.11"]
if floating_address not in floating_ips:
raise exception.FloatingIpNotFoundForAddress(
address=floating_address)
raise exception.FloatingIpNotFoundForAddress(
address=floating_address)

self.stubs.Set(network.api.API, "get_floating_ip_by_address",
network_api_get_floating_ip_by_address)
Expand Down
Expand Up @@ -151,8 +151,9 @@ def test_force_complete_instance_not_found(self):
webob.exc.HTTPNotFound)

def test_force_complete_unexpected_error(self):
self._test_force_complete_failed_with_exception(
exception.NovaException(), webob.exc.HTTPInternalServerError)
self._test_force_complete_failed_with_exception(
exception.NovaException(),
webob.exc.HTTPInternalServerError)


class ServerMigrationsTestsV223(ServerMigrationsTestsV21):
Expand Down
8 changes: 4 additions & 4 deletions nova/tests/unit/api/openstack/compute/test_server_tags.py
Expand Up @@ -327,7 +327,7 @@ def test_delete_non_existing_instance(self):
NON_EXISTING_UUID, TAG1)

def test_delete_all_non_existing_instance(self):
req = self._get_request(
'/v2/fake/servers/%s/tags' % NON_EXISTING_UUID, 'DELETE')
self.assertRaises(exc.HTTPNotFound, self.controller.delete_all,
req, NON_EXISTING_UUID)
req = self._get_request(
'/v2/fake/servers/%s/tags' % NON_EXISTING_UUID, 'DELETE')
self.assertRaises(exc.HTTPNotFound, self.controller.delete_all,
req, NON_EXISTING_UUID)
2 changes: 1 addition & 1 deletion nova/tests/unit/api/openstack/test_wsgi.py
Expand Up @@ -252,7 +252,7 @@ class ResourceTest(MicroversionedTest):
def get_req_id_header_name(self, request):
header_name = 'x-openstack-request-id'
if utils.get_api_version(request) < 3:
header_name = 'x-compute-request-id'
header_name = 'x-compute-request-id'

return header_name

Expand Down

0 comments on commit 3e65f77

Please sign in to comment.