Alon Bar-Lev has posted comments on this change. Change subject: packaging: setup: update firewall for all services ......................................................................
Patch Set 22: (2 comments) .................................................... File packaging/setup/plugins/ovirt-engine-setup/network/firewall_manager.py Line 70: self.environment[osetupcons.CoreEnv.DEVELOPER_MODE] Line 71: or ( Line 72: self.environment[ Line 73: osetupcons.ConfigEnv.AUTOMATIC_FIREWALLING_REQUESTED Line 74: ] is False my priorities: 1. do not treat booleans as non boolean, is False is not something I would like to see. 2. reducing the complexity of the statement. 3. making code self-explainatory, no need for comments. So if you have a condition with a tri-state I would like code to explicitly state so. the first variant meets that. Line 75: ) Line 76: ) Line 77: Line 78: @plugin.event( Line 232: osetupcons.ConfigEnv.FIREWALL_MANAGERS Line 233: ]: Line 234: manager.prepare() Line 235: Line 236: if self._enabled: I think that the set of otopi variables for iptables should be in activate. Line 237: if self.environment[ Line 238: osetupcons.ConfigEnv.FIREWALL_MANAGER Line 239: ] not in [m.name for m in self._available_managers]: Line 240: raise RuntimeError( -- 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: 22 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
