Alon Bar-Lev has posted comments on this change.
Change subject: packaging: setup: update firewall for all services
......................................................................
Patch Set 13:
(4 comments)
....................................................
File packaging/setup/plugins/ovirt-engine-setup/network/firewall_manager.py
Line 162: self._detected_managers.append(manager)
Line 163:
Line 164: if (osetupcons.Const.FIREWALL_MANAGER_FIREWALLD in
Line 165: self._detected_managers and
Line 166: not self.environment[otopicons.NetEnv.FIREWALLD_AVAILABLE]
there is no relationship between 'service' and firewall manager, the detection
can be using service but nobody promise you that:
1. there is service.
2. the service name equals to the manager name.
For all these based on services you can have single statement... please try to
compress the code from start...
for s in (x, y, z):
if service.exists(s)):
available.append(s)
if some_special_service_is_available:
available.append(this special service)
Line 167: ):
Line 168: self._detected_managers.remove(
Line 169: osetupcons.Const.FIREWALL_MANAGER_FIREWALLD
Line 170: )
Line 170: )
Line 171:
Line 172: if (
Line 173: self.environment[osetupcons.ConfigEnv.UPDATE_FIREWALL] is
None and
Line 174: len(self._detected_managers) > 0
so why do you submit new patch?
if not self._detected_managers:
self.environment[osetupcons.ConfigEnv.UPDATE_FIREWALL] = False
if self.environment[osetupcons.ConfigEnv.UPDATE_FIREWALL] is None:
ask user
Line 175: ):
Line 176: self.dialog.note(
Line 177: text=_(
Line 178: 'Setup can automatically configure the firewall '
Line 210: def _customization(self):
Line 211: active_managers = []
Line 212: for manager in self._detected_managers:
Line 213: if self.services.status(manager):
Line 214: active_managers.append(manager)
you do not need to check for 'active' at _customization_is_enabled, you need to
check if there is at least one supported firewall.
the active check belongs to this phase, and should not be based of services.
Line 215:
Line 216: available_managers = (
Line 217: active_managers if active_managers
Line 218: else self._detected_managers
Line 258: osetupcons.ConfigEnv.FIREWALL_MANAGER
Line 259: ] = manager
Line 260: break
Line 261:
Line 262: self.environment[otopicons.NetEnv.IPTABLES_ENABLE] = (
simply do nothing is not good, as there was explicit request.
I thought that you want to ask for firewall update every upgrade, so I do not
understand the above comment.
Line 263: self.environment[
Line 264: osetupcons.ConfigEnv.FIREWALL_MANAGER
Line 265: ] == osetupcons.Const.FIREWALL_MANAGER_IPTABLES
Line 266: )
--
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: 13
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
