-
Notifications
You must be signed in to change notification settings - Fork 23
Add call to super().__init__() where appropriate #21
base: master
Are you sure you want to change the base?
Conversation
@@ -44,7 +44,7 @@ def __init__(self, config, opts, env): | |||
@type env: Environment | |||
""" | |||
self.class_logger.debug("Create AFS object.") | |||
self.opts = opts | |||
super().__init__(config, opts) | |||
|
|||
# Correcting AFS port map | |||
if "mapcorrector" in config: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this:
map_corrector = config.get('mapcorrector', {})
for dev_id, device in mapcorrector.items():
dev_id_int = int(dev_id)
valid_dev_set = {int(key) for key in device}
# remove unnecessary items
portmap = config['portmap']
for items in portmap[:]:
if items[0] == dev_id_int and items[1] not in valid_dev_set:
portmap.remove(items)
# correct port IDs
for items in portmap:
if items[0] == dev_id_int:
items[1] = int(device[str(items[1])]
And I think we can drop lines 68 and 69.
Is there significance to the filtering of config? If yes, then perhaps the GenericEntry should allow descendants to give a set of keys that will be used to filter the config:
class GenericEntry(...):
CONFIG_KEYS_ALLOWED = set()
...
def __init__(...):
...
if self.CONFIG_KEYS_ALLOWED:
self.config = {key: config[key] for key in self.CONFIG_KEYS_ALLOWED}
else:
self.config = config
...
class AFS(...):
CONFIG_KEYS_ALLOWED = {'id', 'instance_type', 'ip_host', 'user', 'password', 'portmap'}
...
self.class_logger.info("Init ONS Switch Cross object.") | ||
self.id = config['id'] | ||
self.type = config['instance_type'] | ||
self.name = config['name'] if "name" in config else "noname" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this self.name = config.get('name', 'noname')
?
Review status: 0 of 7 files reviewed at latest revision, 15 unresolved discussions, some commit checks failed. taf/testlib/dev_onsswcross.py, line 57 at r1 (raw file):
Please change this, and related_conf below, to use taf/testlib/dev_ovscontroller.py, line 69 at r1 (raw file):
Duplicate of assignment in GenericEntry, please remove. taf/testlib/dev_ovscontroller.py, line 70 at r1 (raw file):
Duplicate of assignment in GenericEntry, please remove. taf/testlib/dev_ovscontroller.py, line 606 at r1 (raw file):
Do we need this? It looks like it was removed in other classes that inherit from OvsControllerGeneralMixin. taf/testlib/dev_ovscontroller.py, line 750 at r1 (raw file):
Do we need this? It looks like it was removed in other classes that inherit from OvsControllerGeneralMixin. taf/testlib/dev_ovscontroller.py, line 880 at r1 (raw file):
Do we need this? It looks like it was removed in other classes that inherit from OvsControllerGeneralMixin. taf/testlib/dev_staticcross_ons.py, line 43 at r1 (raw file):
Please make this taf/testlib/dev_staticcross_ons.py, line 48 at r1 (raw file):
Can we make this taf/testlib/dev_vethcross.py, line 52 at r1 (raw file):
Please make this taf/testlib/dev_vethcross.py, line 56 at r1 (raw file):
Can we make this taf/testlib/dev_vlabcross.py, line 55 at r1 (raw file):
Please make this `self.port = config.get('ip_port', '8050') taf/testlib/dev_vlabcross.py, line 58 at r1 (raw file):
Please make this taf/testlib/dev_vlabcross.py, line 62 at r1 (raw file):
Can we make this And similar for the others? Comments from Reviewable |
Reviewed 6 of 7 files at r1. Comments from Reviewable |
Reviewed 1 of 7 files at r1. Comments from Reviewable |
Review scrub: please address comments |
1 similar comment
Review scrub: please address comments |
TAF doctring style is changed. |
Uppon receiving review comments about missing call to super init() I scanned the taf for W0231 warning (super-init-not-called). Pylint has showed this warning at 12 places and I tried to fixed that where I thought it was appropriate.
Have a look if it is worth fixing it.
It is not tested, though.
This change is