The quantum interface isn't implemented as a manager anymore, so this
isn't relevant anymore.

** Changed in: nova
       Status: Confirmed => Invalid

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/887692

Title:
  The QuantumManager could use some refactoring

Status in OpenStack Compute (Nova):
  Invalid

Bug description:
  These changes were suggested by Aaron Lee during his review:

  Patch Set 9: (5 inline comments)

  I'm giving this a 0 because all of my comments are about refactorings
  that would have made it easier for me to read what this patch is
  doing. The functionality looks good, as far as I can tell, but I think
  the code needs to be cleaner.

  ....................................................
  File nova/network/linux_net.py
  Line 995:         # Don't forward traffic unless we were told to be a gateway
  I would rather see the actions within the conditional more focused on the 
decision of the conditional.

  i.e.
  guard = "DROP"
  if gateway
   guard = "ACCEPT"

  for direction in ['in', 'out']
     iptables_manager.ipv4['filter'].add_rule('FORWARD',
                                      '--$(direction)-interface %(guard)s -j 
%(bridge)s' % \
                                      locals())

  This will make it much easier to fix errors in the generation of those
  commands.

  ....................................................
  File nova/network/quantum/manager.py
  Line 213:                     "label": "quantum-net-%s" % quantum_net_id}
  I don't think this get network ref logic is a part of allocating for an 
instance. It would be better stored in it's own method.

  Line 234:         LOG.info("Using DHCP for network: %s" % 
network_ref['label'])
  This would be much easier to maintain and test in small units if each step 
was a method, and enable_dhcp just called each individual method. Refactoring 
like that would also make more granular error handling easier.

  Line 449:                 subnet['network_id'], project_id)
  It appears the real meat of this function is these three calls to the driver, 
it should be separated from populating the network_ref.

  This data mutating done on this response from get_subnets_by_net_id is
  very similar to the changes made to this data on both other calls to
  get_subnets_by_net_id. This is another candidate for refactoring, but
  may be out of the scope of this commit.

  Line 485:     def get_dhcp_hosts_text(self, context, subnet_id, 
project_id=None):
  These methods share 10 lines, maybe the collection of information and 
iteration over that list could be abstracted?

  There are a few others I wanted to do as well.

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/887692/+subscriptions

-- 
Mailing list: https://launchpad.net/~yahoo-eng-team
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~yahoo-eng-team
More help   : https://help.launchpad.net/ListHelp

Reply via email to