Is this truly necessary? Test/stubbing code should only be in tests, not in module code. Is this something we might look at quickly while doing this re-factoring?
239 + """Constant to indicate whether this manager requires VIF to create a
240 + VLAN tag."""
I think the convention is to have this be a comment with # not """, so maybe something like:
# If True, this manager requires VIF to create VLAN tag
SHOULD_CREATE_VLAN = False
Regardless, the "Constant to indicate whether" is superfluous and its removal would shorten these and help readability either way.
957 + try:
958 + for (network, mapping) in network_info:
959 + self.vif_driver.unplug(instance, network, mapping)
960 + except:
961 + LOG.warning(_("Failed while unplugging vif of instance '%s'"),
962 + instance['name'])
963 + raise
964 +
I'm dubious as to the necessity of this try/except. If you're only logging, it would be better IMO to rely on good error handling in the self.vif_driver.unplug method and not in this one. The same thing happens on lines 1799-1807.
Can we maybe shorten 'libvirt_ovs_integration_bridge' to just 'libvirt_ovs_bridge'? I'm not in love with having such long flag names, and we can always clarify in the description string.
44 + if not FLAGS.stub_network:
Is this truly necessary? Test/stubbing code should only be in tests, not in module code. Is this something we might look at quickly while doing this re-factoring?
239 + """Constant to indicate whether this manager requires VIF to create a
240 + VLAN tag."""
I think the convention is to have this be a comment with # not """, so maybe something like:
# If True, this manager requires VIF to create VLAN tag
SHOULD_CREATE_VLAN = False
Regardless, the "Constant to indicate whether" is superfluous and its removal would shorten these and help readability either way.
957 + try: driver. unplug( instance, network, mapping) _("Failed while unplugging vif of instance '%s'"),
958 + for (network, mapping) in network_info:
959 + self.vif_
960 + except:
961 + LOG.warning(
962 + instance['name'])
963 + raise
964 +
I'm dubious as to the necessity of this try/except. If you're only logging, it would be better IMO to rely on good error handling in the self.vif_ driver. unplug method and not in this one. The same thing happens on lines 1799-1807.
1158 +# Copyright (C) 2011 Midokura KK
1159 +# Copyright (C) 2011 Nicira, Inc
I'm not certain, but I believe code belongs to OpenStack, LLC?
1187 +flags. DEFINE_ string( 'libvirt_ ovs_integration _bridge' , 'br-int',
Can we maybe shorten 'libvirt_ ovs_integration _bridge' to just 'libvirt_ ovs_bridge' ? I'm not in love with having such long flag names, and we can always clarify in the description string.