openstack
/
neutron
Code Issues Proposed changes

Fix some new pylint "E" warnings 

After updating pylint, it started emitting additional "E"
warnings in some cases, fix them.

  unsubscriptable-object,
  unsupported-delete-operation

These were associated with the OVN AgentCache code. Instead
of using a subscript, create get/delete methods to do the
same thing.

  used-before-assignment

Re-factor some code so it's clear to pylint variables are
being assigned properly.

Trivialfix

Change-Id: I4a5ccb7f33465705e59b5274c41db3c371862b1e
This commit is contained in:
Brian Haley 2023-04-29 11:02:38 -04:00
parent 7f460ab1af
commit 126d54badc
9 changed files with 56 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.pylintrc
View File

@ -28,9 +28,6 @@ disable=
no-method-argument,
no-self-argument,
not-an-iterable,
unsubscriptable-object,
unsupported-delete-operation,
used-before-assignment,
# "W" Warnings for stylistic problems or minor programming issues
abstract-method,
arguments-differ,

5
neutron/agent/linux/dhcp.py
View File

@ -391,10 +391,9 @@ class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
with open(file_name, 'r') as f:
return converter(f.read()) if converter else f.read()
except ValueError:
msg = "Unable to convert value in %s"
LOG.debug("Unable to convert value in %s", file_name)
except IOError:
msg = "Unable to access %s"
LOG.debug(msg, file_name)
LOG.debug("Unable to access %s", file_name)
return None
@property

2
neutron/db/l3_db.py
View File

@ -1504,6 +1504,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
self._core_plugin, context.elevated(),
{'port': port}, check_allow_post=False)
dns_data = None
with plugin_utils.delete_port_on_error(
self._core_plugin, context.elevated(),
external_port['id']),\
@ -1587,6 +1588,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
# raise the underlying exception
raise e.errors[0].error
dns_data = None
fip = floatingip['floatingip']
with db_api.CONTEXT_WRITER.using(context):
floatingip_obj = self._get_floatingip(context, id)

2
neutron/db/l3_hamode_db.py
View File

@ -503,6 +503,8 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin,
self._core_plugin.delete_network(admin_ctx, net.network_id)
def safe_delete_ha_network(self, context, ha_network, tenant_id):
# initialize to silence pylint used-before-assignment warning
net_id = None
try:
# reference the attr inside the try block before we attempt
# to delete the network and potentially invalidate the

10
neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py
View File

@ -195,7 +195,7 @@ class MetadataAgent(NeutronAgent):
# If ovn-controller is down, then metadata agent is down even
# if the metadata-agent binary is updating external_ids.
try:
if not AgentCache()[self.chassis_private.name].alive:
if not AgentCache().get(self.chassis_private.name).alive:
return False
except KeyError:
return False
@ -230,7 +230,7 @@ class OVNNeutronAgent(NeutronAgent):
# If ovn-controller is down, then OVN Neutron Agent is down even
# if the neutron-ovn-agent binary is updating external_ids.
try:
if not AgentCache()[self.chassis_private.name].alive:
if not AgentCache().get(self.chassis_private.name).alive:
return False
except KeyError:
return False
@ -257,7 +257,7 @@ class OVNNeutronAgent(NeutronAgent):
@utils.SingletonDecorator
class AgentCache:
class AgentCache(object):
def __init__(self, driver=None):
# This is just to make pylint happy because it doesn't like calls to
# AgentCache() with no arguments, despite init only being called the
@ -273,7 +273,7 @@ class AgentCache:
_agents = copy.copy(self.agents)
return iter(_agents.values())
def __getitem__(self, key):
def get(self, key):
return self.agents[key]
def update(self, agent_type, row, clear_down=False):
@ -286,7 +286,7 @@ class AgentCache:
self.agents[agent.agent_id] = agent
return agent
def __delitem__(self, agent_id):
def delete(self, agent_id):
del self.agents[agent_id]
def agents_by_chassis_private(self, chassis_private):

7
neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py
View File

@ -1359,7 +1359,7 @@ def get_agents(self, context, filters=None, fields=None, _driver=None):
def get_agent(self, context, id, fields=None, _driver=None):
try:
return n_agent.AgentCache()[id].as_dict()
return n_agent.AgentCache().get(id).as_dict()
except KeyError:
raise n_exc.agent.AgentNotFound(id=id)
@ -1418,6 +1418,11 @@ def delete_agent(self, context, id, _driver=None):
'SB_Global', '.', 'external_ids', delete_agent=str(id),
if_exists=True).execute(check_error=True)
try:
n_agent.AgentCache().delete(id)
except KeyError:
LOG.debug('OVN agent %s has been deleted concurrently', id)
def get_availability_zones(cls, context, _driver, filters=None, fields=None,
sorts=None, limit=None, marker=None,

2
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py
View File

@ -327,7 +327,7 @@ class ChassisAgentDeleteEvent(ChassisAgentEvent):
return False
def run(self, event, row, old):
del n_agent.AgentCache()[row.external_ids['delete_agent']]
n_agent.AgentCache().delete([row.external_ids['delete_agent']])
class ChassisAgentWriteEvent(ChassisAgentEvent):

63
neutron/profiling/profiled_decorator.py
View File

@ -42,41 +42,40 @@ def profile(f):
@functools.wraps(f)
def profile_wrapper(*args, **kwargs):
if not cfg.CONF.enable_code_profiling:
return f(*args, **kwargs)
profid = "%s.%s" % (f.__module__, f.__name__)
profiler = cProfile.Profile()
start_time = datetime.now()
try:
if cfg.CONF.enable_code_profiling:
profid = "%s.%s" % (f.__module__, f.__name__)
profiler = cProfile.Profile()
profiler.enable()
start_time = datetime.now()
profiler.enable()
return f(*args, **kwargs)
finally:
if cfg.CONF.enable_code_profiling:
profiler.disable()
elapsed_time = datetime.now() - start_time
elapsed_time_ms = (elapsed_time.seconds * 1000.0 +
elapsed_time.microseconds / 1000.0)
stream = io.StringIO()
stats = pstats.Stats(profiler, stream=stream).sort_stats(
SORT_BY)
stats.print_stats(cfg.CONF.code_profiling_calls_to_log)
stats.print_callers(cfg.CONF.code_profiling_calls_to_log)
stats.print_callees(cfg.CONF.code_profiling_calls_to_log)
profiler_info = utils.collect_profiler_info()
if not profiler_info:
profiler_info = {'base_id': 'not available',
'parent_id': 'not available'}
LOG.debug('os-profiler parent trace-id %(parent_id)s '
'trace-id %(trace_id)s %(elapsed_time)7d millisecs '
'elapsed for %(method)s(*%(args)s, **%(kwargs)s):'
'\n%(profiler_data)s',
{'parent_id': profiler_info['parent_id'],
'trace_id': profiler_info['base_id'],
'elapsed_time': elapsed_time_ms,
'method': profid,
'args': args,
'kwargs': kwargs,
'profiler_data': stream.getvalue()})
stream.close()
profiler.disable()
elapsed_time = datetime.now() - start_time
elapsed_time_ms = (elapsed_time.seconds * 1000.0 +
elapsed_time.microseconds / 1000.0)
stream = io.StringIO()
stats = pstats.Stats(profiler, stream=stream).sort_stats(SORT_BY)
stats.print_stats(cfg.CONF.code_profiling_calls_to_log)
stats.print_callers(cfg.CONF.code_profiling_calls_to_log)
stats.print_callees(cfg.CONF.code_profiling_calls_to_log)
profiler_info = utils.collect_profiler_info()
if not profiler_info:
profiler_info = {'base_id': 'not available',
'parent_id': 'not available'}
LOG.debug('os-profiler parent trace-id %(parent_id)s '
'trace-id %(trace_id)s %(elapsed_time)7d millisecs '
'elapsed for %(method)s(*%(args)s, **%(kwargs)s):'
'\n%(profiler_data)s',
{'parent_id': profiler_info['parent_id'],
'trace_id': profiler_info['base_id'],
'elapsed_time': elapsed_time_ms,
'method': profid,
'args': args,
'kwargs': kwargs,
'profiler_data': stream.getvalue()})
stream.close()
return profile_wrapper

14
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py
View File

@ -498,18 +498,18 @@ class TestAgentMonitor(base.TestOVNFunctionalBase):
def test_agent_change_controller(self):
self.assertEqual(neutron_agent.ControllerGatewayAgent,
type(neutron_agent.AgentCache()[self.chassis_name]))
type(neutron_agent.AgentCache().get(self.chassis_name)))
self.sb_api.db_set('Chassis', self.chassis_name, ('other_config',
{'ovn-cms-options': ''})).execute(check_error=True)
n_utils.wait_until_true(lambda:
neutron_agent.AgentCache()[self.chassis_name].
neutron_agent.AgentCache().get(self.chassis_name).
chassis.other_config['ovn-cms-options'] == '')
self.assertEqual(neutron_agent.ControllerAgent,
type(neutron_agent.AgentCache()[self.chassis_name]))
type(neutron_agent.AgentCache().get(self.chassis_name)))
def test_agent_updated_at_use_nb_cfg_timestamp(self):
def check_agent_ts():
agent = neutron_agent.AgentCache()[self.chassis_name]
agent = neutron_agent.AgentCache().get(self.chassis_name)
chassis_ts = self.sb_api.db_get(
'Chassis_Private', self.chassis_name,
'nb_cfg_timestamp').execute(check_error=True)
@ -532,7 +532,7 @@ class TestAgentMonitor(base.TestOVNFunctionalBase):
try:
n_utils.wait_until_true(check_agent_ts, timeout=5)
except n_utils.WaitTimeout:
agent = neutron_agent.AgentCache()[self.chassis_name]
agent = neutron_agent.AgentCache().get(self.chassis_name)
chassis_ts = self.sb_api.db_get(
'Chassis_Private', self.chassis_name,
'nb_cfg_timestamp').execute(check_error=True)
@ -541,14 +541,14 @@ class TestAgentMonitor(base.TestOVNFunctionalBase):
def test_agent_restart(self):
def check_agent_up():
agent = neutron_agent.AgentCache()[self.chassis_name]
agent = neutron_agent.AgentCache().get(self.chassis_name)
return agent.alive
def check_agent_down():
return not check_agent_up()
def check_nb_cfg_timestamp_is_not_null():
agent = neutron_agent.AgentCache()[self.chassis_name]
agent = neutron_agent.AgentCache().get(self.chassis_name)
return agent.updated_at != 0
if not self.sb_api.is_table_present('Chassis_Private'):