Alon Bar-Lev has posted comments on this change.
Change subject: packaging: setup: update firewall for all services
......................................................................
Patch Set 15:
(7 comments)
....................................................
File packaging/setup/ovirt_engine_setup/firewall_manager_base.py
Line 19: from otopi import util
Line 20:
Line 21:
Line 22: @util.export
Line 23: class FirewallManagerBase(object):
if it is used only be single module there is no reason to expose.
Line 24:
Line 25: def __init__(self, plugin):
Line 26: self._plugin = plugin
Line 27:
Line 23: class FirewallManagerBase(object):
Line 24:
Line 25: def __init__(self, plugin):
Line 26: self._plugin = plugin
Line 27:
I think another property can be plugin to make use of plugin nicer.
You can also add property of environment like in other wrapper we have.
Line 28: @property
Line 29: def name(self):
Line 30: raise RuntimeError('Unset')
Line 31:
....................................................
File packaging/setup/plugins/ovirt-engine-setup/network/firewall_manager.py
Line 38: from ovirt_engine_setup import util as osetuputil
Line 39: from ovirt_engine_setup import firewall_manager_base
Line 40:
Line 41:
Line 42: class IpTablesManager(firewall_manager_base.FirewallManagerBase):
please collapse these into plugin module, they are not exposed out of the scope
of the manager, please add _ prefix as they are private.
Line 43:
Line 44: _SERVICE = 'iptables'
Line 45:
Line 46: def __init__(self, plugin):
Line 161: )
Line 162: )
Line 163: self.environment[osetupcons.ConfigEnv.FIREWALL_MANAGERS] = [
Line 164: FirewalldManager(self),
Line 165: IpTablesManager(self),
plugin=self
Line 166: ]
Line 167: self.environment.setdefault(
Line 168: osetupcons.NetEnv.FIREWALLD_SERVICES,
Line 169: []
Line 192: osetupcons.Stages.DIALOG_TITLES_S_NETWORK,
Line 193: ),
Line 194: )
Line 195: def _customization_is_enabled(self):
Line 196: self._supported_managers = [
this is now confusing, as the supported managers are the manager we actually
support which are FIREWALL_MANAGERS.
need to find other name for this, candidates? enabled?
in any case, I do not want to hold string from this point on... I would like a
collection of managers objects.
Line 197: x.strip()
Line 198: for x in self.environment[
Line 199: osetupcons.ConfigEnv.SUPPORTED_FIREWALL_MANAGERS
Line 200: ].split(',')
Line 208: manager.name,
Line 209: manager.detect(),
Line 210: manager.name in self._supported_managers,
Line 211: ),
Line 212: )
so you call detect twice, and condition twice?
Line 213: if (
Line 214: manager.name in self._supported_managers and
Line 215: manager.detect()
Line 216: ):
Line 459:
example=osetupcons.FileLocations.OVIRT_IPTABLES_EXAMPLE
Line 460: )
Line 461: )
Line 462:
Line 463: if (
not sure if this should not be added to the object...
so we:
for m in self._xxx_managers:
m.print_usage()
Line 464: osetupcons.Const.FIREWALL_MANAGER_FIREWALLD in
Line 465: self._supported_managers
Line 466: ):
Line 467: commands = []
--
To view, visit http://gerrit.ovirt.org/20737
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If3c1a634b2e8539ebd604205b5487290c8d8a1a9
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
