)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"85b2319f053db7920b7ca7d0d47e55221c5dc57f","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yves-Gwenael Bourhis \u003cyves-gwenael.bourhis@cloudwatt.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-11-29 10:24:31 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make dnsmaq aware of all names"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Each dnsmasq instance on a network is not aware of other dnsmasq\u0027s leases."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"AAAATn%2F96NI%3D","line":7,"updated":"2013-12-04 16:33:45.000000000","message":"s/dnsmaq/dnsmasq/","commit_id":"75a0037e1b8d214ee310e276dde4eea6c163a39b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"85b2319f053db7920b7ca7d0d47e55221c5dc57f","unresolved":false,"context_lines":[{"line_number":12,"context_line":"file, it can resolve only the hosts to which it gives a dhcp lease and no more."},{"line_number":13,"context_line":"i.e.:"},{"line_number":14,"context_line":"If dnsmasq service n°1 gives a lease to instance n°1, and dnsmasq service n°2"},{"line_number":15,"context_line":"gives a lease to instance n°2, both VM instances and dnsmasq services beeing on"},{"line_number":16,"context_line":"the same network: instance n°1 can not resolve instance n°2, because instance"},{"line_number":17,"context_line":"n°1 queries dnsmasq n°1, and since it did not give the lease to instance n°2,"},{"line_number":18,"context_line":"it can not resolve it (it is not aware of its existance). Same issue if"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"AAAATn%2F96Lw%3D","line":15,"updated":"2013-12-04 16:33:45.000000000","message":"s/beeing/being/","commit_id":"75a0037e1b8d214ee310e276dde4eea6c163a39b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"85b2319f053db7920b7ca7d0d47e55221c5dc57f","unresolved":false,"context_lines":[{"line_number":15,"context_line":"gives a lease to instance n°2, both VM instances and dnsmasq services beeing on"},{"line_number":16,"context_line":"the same network: instance n°1 can not resolve instance n°2, because instance"},{"line_number":17,"context_line":"n°1 queries dnsmasq n°1, and since it did not give the lease to instance n°2,"},{"line_number":18,"context_line":"it can not resolve it (it is not aware of its existance). Same issue if"},{"line_number":19,"context_line":"instance n°2 tries to resolve instance n°1."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The solution is to provide dnsmasq with an --addn-hosts file of all hosts on"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"AAAATn%2F96LU%3D","line":18,"updated":"2013-12-04 16:33:45.000000000","message":"nit: *existence","commit_id":"75a0037e1b8d214ee310e276dde4eea6c163a39b"}],"neutron/agent/linux/dhcp.py":[{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"change_message_id":"a73db87e98bde285d8cacbf2c435d42094d1b84a","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            self.NEUTRON_NETWORK_ID_KEY: self.network.id,"},{"line_number":305,"context_line":"        }"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"        dhcp_hostsfile, addn_hosts \u003d self._output_hosts_files()"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        cmd \u003d ["},{"line_number":310,"context_line":"            \u0027dnsmasq\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2FDt8%3D","line":307,"updated":"2013-11-15 02:09:18.000000000","message":"IMO, we should have two method for better readability.\n_output_dhcp_hostsfine\n_output_addn_hosts file","commit_id":"6c8888c24cccee8424fa15ea3172cfd75b45ad3a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"2102743dce491e80e95a39922d99f3625ddf2b5d","unresolved":false,"context_lines":[{"line_number":416,"context_line":"                name \u003d \u0027%s.%s\u0027 % (host_name, self.conf.dhcp_domain)"},{"line_number":417,"context_line":"                yield (port, alloc, host_name, name)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def _output_dhcp_hosts_file(self):"},{"line_number":420,"context_line":"        \"\"\"Writes a dnsmasq compatible dhcp hosts file.\"\"\""},{"line_number":421,"context_line":"        dhcp_buf \u003d StringIO.StringIO()"},{"line_number":422,"context_line":"        for (port, alloc, host_name, name) in self._iter_hosts():"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F%2FBg0%3D","line":419,"updated":"2013-11-15 15:14:11.000000000","message":"This patch would be more concise if this method were left unchanged and a new method, _output_addn_hosts_file simply added to the file.\n\nIt could be changed slightly to make use of the _iter_hosts method but please avoid other, gratuitous, changes like changing the name of the method and changing buf to dhcp_buf, etc.","commit_id":"b1b6c5699723e29a566200a7a505695d8d6beec8"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"2102743dce491e80e95a39922d99f3625ddf2b5d","unresolved":false,"context_lines":[{"line_number":417,"context_line":"                yield (port, alloc, host_name, name)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def _output_dhcp_hosts_file(self):"},{"line_number":420,"context_line":"        \"\"\"Writes a dnsmasq compatible dhcp hosts file.\"\"\""},{"line_number":421,"context_line":"        dhcp_buf \u003d StringIO.StringIO()"},{"line_number":422,"context_line":"        for (port, alloc, host_name, name) in self._iter_hosts():"},{"line_number":423,"context_line":"            set_tag \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F%2FBec%3D","line":420,"updated":"2013-11-15 15:14:11.000000000","message":"Note that I have another patch in flight that changes this method also.  That is okay.  I just wanted to make you aware of it in case it gets merged first this one will need to be rebased to it and probably adjusted a little to avoid conflict (and vice versa).\n\nhttps://review.openstack.org/#/c/56263/","commit_id":"b1b6c5699723e29a566200a7a505695d8d6beec8"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"2102743dce491e80e95a39922d99f3625ddf2b5d","unresolved":false,"context_lines":[{"line_number":437,"context_line":"                        port.mac_address, name, alloc.ip_address,"},{"line_number":438,"context_line":"                    )"},{"line_number":439,"context_line":"                )"},{"line_number":440,"context_line":"        dhcp_hosts \u003d self.get_conf_file_name(\u0027dhcp_hosts\u0027)"},{"line_number":441,"context_line":"        utils.replace_file(dhcp_hosts, dhcp_buf.getvalue())"},{"line_number":442,"context_line":"        return dhcp_hosts"},{"line_number":443,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F%2FBe0%3D","line":440,"updated":"2013-11-15 15:14:11.000000000","message":"Please don\u0027t change the name of this file from \"host\" to \"dhcp_hosts.\"","commit_id":"b1b6c5699723e29a566200a7a505695d8d6beec8"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"2102743dce491e80e95a39922d99f3625ddf2b5d","unresolved":false,"context_lines":[{"line_number":450,"context_line":"        utils.replace_file(addn_hosts, addn_buf.getvalue())"},{"line_number":451,"context_line":"        return addn_hosts"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    def _output_hosts_files(self):"},{"line_number":454,"context_line":"        return (self._output_dhcp_hosts_file(), self._output_addn_hosts_file())"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"    def _output_opts_file(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F%2FBfI%3D","line":453,"updated":"2013-11-15 15:14:11.000000000","message":"This method adds no value.  It no longer needs to return anything.  (I know it did return something in previous patches.)\n\nJust call the two methods above where this method is called in this patch and remove this method.","commit_id":"b1b6c5699723e29a566200a7a505695d8d6beec8"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"21211cb977330dfa85f6e02bedaa8de634b1641d","unresolved":false,"context_lines":[{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                buf.write(\u0027%s,%s,%s\\n\u0027 %"},{"line_number":434,"context_line":"                          (port.mac_address, name, alloc.ip_address))"},{"line_number":435,"context_line":"        hosts \u003d self.get_conf_file_name(\u0027dhcp_hosts\u0027)"},{"line_number":436,"context_line":"        utils.replace_file(hosts, buf.getvalue())"},{"line_number":437,"context_line":"        return hosts"},{"line_number":438,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2FA8Q%3D","line":435,"updated":"2013-11-15 17:35:14.000000000","message":"Please don\u0027t change the names here.  Especially not the name of the file.  We are all used to looking for the file named \u0027host\u0027 on the filesystem.  I don\u0027t think that changing it is necessary.\n\nAlso, if these three lines matched the 3 in the base then this patch would read better.  (I know that \"name\" is overloaded here but it doesn\u0027t cause a problem and my patch will change that soon enough.)","commit_id":"c627fd0e70021c43c6441d2f13c77e23ec34c86f"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"21211cb977330dfa85f6e02bedaa8de634b1641d","unresolved":false,"context_lines":[{"line_number":441,"context_line":"        buf \u003d StringIO.StringIO()"},{"line_number":442,"context_line":"        for (port, alloc, host_name, name) in self._iter_hosts():"},{"line_number":443,"context_line":"            buf.write(\u0027%s\\t%s %s\\n\u0027 % (alloc.ip_address, host_name, name))"},{"line_number":444,"context_line":"        addn_hosts \u003d self.get_conf_file_name(\u0027dns_hosts\u0027)"},{"line_number":445,"context_line":"        utils.replace_file(addn_hosts, buf.getvalue())"},{"line_number":446,"context_line":"        return addn_hosts"},{"line_number":447,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2FA8I%3D","line":444,"updated":"2013-11-15 17:35:14.000000000","message":"Consider naming this file \u0027addn_hosts\u0027.  For those who are familiar with the dnsmasq options, it will be natural to look for the file by this name.","commit_id":"c627fd0e70021c43c6441d2f13c77e23ec34c86f"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"85b2319f053db7920b7ca7d0d47e55221c5dc57f","unresolved":false,"context_lines":[{"line_number":433,"context_line":"    def _output_hosts_file(self):"},{"line_number":434,"context_line":"        \"\"\"Writes a dnsmasq compatible dhcp hosts file."},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"        The generated file is send to the --dhcp-hostsfile option of dnsmasq,"},{"line_number":437,"context_line":"        and lists the hosts on the network which should receive a dhcp lease."},{"line_number":438,"context_line":"        Each line in this file is in the form::"},{"line_number":439,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F96Hc%3D","line":436,"updated":"2013-12-04 16:33:45.000000000","message":"nit: *sent","commit_id":"75a0037e1b8d214ee310e276dde4eea6c163a39b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"85b2319f053db7920b7ca7d0d47e55221c5dc57f","unresolved":false,"context_lines":[{"line_number":466,"context_line":"    def _output_addn_hosts_file(self):"},{"line_number":467,"context_line":"        \"\"\"Writes a dnsmasq compatible additional hosts file."},{"line_number":468,"context_line":""},{"line_number":469,"context_line":"        The generated file is send to the --addn-hosts option of dnsmasq,"},{"line_number":470,"context_line":"        and lists the hosts on the network which should be resolved even if"},{"line_number":471,"context_line":"        the dnsmaq instance did not give a lease to the host (see the"},{"line_number":472,"context_line":"        `_output_hosts_file` method)."}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F96HI%3D","line":469,"updated":"2013-12-04 16:33:45.000000000","message":"nit: *sent","commit_id":"75a0037e1b8d214ee310e276dde4eea6c163a39b"},{"author":{"_account_id":7141,"name":"Sylvain Afchain","email":"safchain@redhat.com","username":"sylvainA"},"change_message_id":"16678b5f014eb7ff6741c653533c2176817ac226","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        \"\"\""},{"line_number":476,"context_line":"        buf \u003d StringIO.StringIO()"},{"line_number":477,"context_line":"        for (port, alloc, host_name, name) in self._iter_hosts():"},{"line_number":478,"context_line":"            buf.write(\u0027%s\\t%s %s\\n\u0027 % (alloc.ip_address, host_name, name))"},{"line_number":479,"context_line":"        addn_hosts \u003d self.get_conf_file_name(\u0027addn_hosts\u0027)"},{"line_number":480,"context_line":"        utils.replace_file(addn_hosts, buf.getvalue())"},{"line_number":481,"context_line":"        return addn_hosts"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F9g%2FQ%3D","line":478,"updated":"2013-12-10 17:52:26.000000000","message":"nit: It seems the first \\t could be replaced by a space ?","commit_id":"dbe6b4d4164fce0645a493dde31451eeefd507ba"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4d54b3eb5d7a20b15649279e743ff0b1850192fb","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        \"\"\""},{"line_number":476,"context_line":"        buf \u003d StringIO.StringIO()"},{"line_number":477,"context_line":"        for (port, alloc, host_name, name) in self._iter_hosts():"},{"line_number":478,"context_line":"            # It is important to have the full `name` (host_name.domain) before"},{"line_number":479,"context_line":"            # the host_name, if we want the domain to appear in PTR responses."},{"line_number":480,"context_line":"            # For this reason, the first name after the IP has to be the"},{"line_number":481,"context_line":"            # FQDN."}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAUH%2F%2B0qg%3D","line":478,"updated":"2014-01-21 18:31:31.000000000","message":"This comment is a bit verbose.  Maybe it is enough to just say the order has to be like this without all the history.","commit_id":"8ad97675f5695f635f9f7b73d760fac8a9fc0be0"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"482caeebb7d120aa0a9ece6200f502e7707c6b08","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            \u0027--pid-file\u003d%s\u0027 % self.get_conf_file_name("},{"line_number":327,"context_line":"                \u0027pid\u0027, ensure_conf_dir\u003dTrue),"},{"line_number":328,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":329,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":330,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":331,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":332,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUX%2F%2FqUQ%3D","line":329,"updated":"2014-02-11 17:22:42.000000000","message":"I am slightly concerned about the extra overhead that this line adds to the spawning process of dnsmasq, and I wonder if this might have repercussions on the overall performance of the process of providing dhcp to tenant VM\u0027s. During your patch you effectively kept the enumeration over network.ports; this is done already twice at line 555 and 447. Now you\u0027re adding a third at line 495. Is there a way we can do better (e.g. once)? Or at least not add further processing overhead? Potentially a network may have thousands of ports.","commit_id":"d6019a4c15fe964e4db63872aae25b5a11f2c213"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"69496394f9c6049d70723c7051955c8113457f9b","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            \u0027--pid-file\u003d%s\u0027 % self.get_conf_file_name("},{"line_number":327,"context_line":"                \u0027pid\u0027, ensure_conf_dir\u003dTrue),"},{"line_number":328,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":329,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":330,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":331,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":332,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUX%2F%2Fgn0%3D","line":329,"in_reply_to":"AAAAUX%2F%2Fi6w%3D","updated":"2014-02-12 17:24:50.000000000","message":"sounds good, thanks!","commit_id":"d6019a4c15fe964e4db63872aae25b5a11f2c213"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"933cc6eb90ace2844516548f7875e0beda94d050","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            \u0027--pid-file\u003d%s\u0027 % self.get_conf_file_name("},{"line_number":327,"context_line":"                \u0027pid\u0027, ensure_conf_dir\u003dTrue),"},{"line_number":328,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":329,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":330,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":331,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":332,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUX%2F%2FpXo%3D","line":329,"in_reply_to":"AAAAUX%2F%2Fp7c%3D","updated":"2014-02-11 19:45:53.000000000","message":"Hi Yves, I agree with Nachi\u0027s comment; separating concerns aids maintainability and readability. That said, with the code as it is, it\u0027s obviously difficult to achieve both. This is because the logic of gathering port information, formatting output and writing to files is all tight together. If that was broken down into separate bits things would be a lot easier. For example, you could have a method that emits just port information, one that filters information out, one that formats the output according to what\u0027s needed and one that blindly writes to a file. These can be combined together to achieve the overall goal of generating the files needed not at speed\u0027s expenses. I recognize that\u0027s a lot to visualize but you were kind on the right track with the introduction of _iter_hosts. Please let me know whether you feel comfortable going down this optimization road otherwise, we could consider it an enhancement.","commit_id":"d6019a4c15fe964e4db63872aae25b5a11f2c213"},{"author":{"_account_id":8674,"name":"Yves-Gwenael Bourhis","email":"ygbourhis@gmail.com","username":"yves"},"change_message_id":"43154df49c1b2d08791144142039098f06f975f0","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            \u0027--pid-file\u003d%s\u0027 % self.get_conf_file_name("},{"line_number":327,"context_line":"                \u0027pid\u0027, ensure_conf_dir\u003dTrue),"},{"line_number":328,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":329,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":330,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":331,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":332,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUX%2F%2Fi6w%3D","line":329,"in_reply_to":"AAAAUX%2F%2FpXo%3D","updated":"2014-02-12 13:20:50.000000000","message":"Hi Armando,\nIf you agree with this, I submitted https://bugs.launchpad.net/neutron/+bug/1279334 and assigned it to myself, because we need this current fix quite fast, and I will straight away work on the performance enhancement you propose.\nIs that OK for you?","commit_id":"d6019a4c15fe964e4db63872aae25b5a11f2c213"},{"author":{"_account_id":8674,"name":"Yves-Gwenael Bourhis","email":"ygbourhis@gmail.com","username":"yves"},"change_message_id":"f0d2769cd10c4ff322a84f3826e7c8b10749d531","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            \u0027--pid-file\u003d%s\u0027 % self.get_conf_file_name("},{"line_number":327,"context_line":"                \u0027pid\u0027, ensure_conf_dir\u003dTrue),"},{"line_number":328,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":329,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":330,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":331,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":332,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAUX%2F%2Fp7c%3D","line":329,"in_reply_to":"AAAAUX%2F%2FqUQ%3D","updated":"2014-02-11 18:10:43.000000000","message":"Hi Armando.\n\nAs you can see per the inline comment in my patch set 5, there was only one iteration but I was requested to split the code into multiple methods.\n\nIt is not possible to iterate once without having a unique method for all host files.\n\nAlso, we iterate on a DictModel instance (already stored in RAM without any API calls), so a cache would be useless (it would even be worse by doubling the RAM usage for ports).","commit_id":"d6019a4c15fe964e4db63872aae25b5a11f2c213"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"361aa1c6a9836691269853ffb70935c44ad02a67","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        LOG.debug(_(\u0027Reloading allocations for network: %s\u0027), self.network.id)"},{"line_number":402,"context_line":"        self.device_manager.update(self.network)"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"    def _iter_hosts(self):"},{"line_number":405,"context_line":"        \"\"\"Iterate over hosts."},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        For each host on the network we yield a tuple containing:"}],"source_content_type":"text/x-python","patch_set":22,"id":"AAAAWH%2F%2BgtY%3D","line":404,"updated":"2014-03-26 09:47:52.000000000","message":"nit: it\u0027s usually more readable to put functions in top-down order, so when you read _iter_hosts after _output_hosts_file and _output_addn_hosts_file the reader has more context.","commit_id":"f0821712cb29f1c0c2a7f14c9a25ffcd115eacfe"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"361aa1c6a9836691269853ffb70935c44ad02a67","unresolved":false,"context_lines":[{"line_number":412,"context_line":"            name,  # Host name and domain name in the format \u0027hostname.domain\u0027."},{"line_number":413,"context_line":"        )"},{"line_number":414,"context_line":"        \"\"\""},{"line_number":415,"context_line":"        r \u003d re.compile(\u0027[:.]\u0027)"},{"line_number":416,"context_line":"        for port in self.network.ports:"},{"line_number":417,"context_line":"            for alloc in port.fixed_ips:"},{"line_number":418,"context_line":"                hostname \u003d \u0027host-%s\u0027 % r.sub(\u0027-\u0027, alloc.ip_address)"}],"source_content_type":"text/x-python","patch_set":22,"id":"AAAAWH%2F%2BguE%3D","line":415,"updated":"2014-03-26 09:47:52.000000000","message":"performance warning, probably not very important here.\n\nWe have recently identified the \"re.compile\"\u0027s as one of the *MOST* time-consuming\nprocesses into the agents. We found it via profiling.\n\nI think it would be good to put this on the constructor, and then use the compiled regular expression as self.r (r-\u003esome more explanatory name).\n\n\"r\" is not very self explanatory.","commit_id":"f0821712cb29f1c0c2a7f14c9a25ffcd115eacfe"},{"author":{"_account_id":8674,"name":"Yves-Gwenael Bourhis","email":"ygbourhis@gmail.com","username":"yves"},"change_message_id":"7fc27e67e516732a81813e3e33914abe1be325ed","unresolved":false,"context_lines":[{"line_number":412,"context_line":"            name,  # Host name and domain name in the format \u0027hostname.domain\u0027."},{"line_number":413,"context_line":"        )"},{"line_number":414,"context_line":"        \"\"\""},{"line_number":415,"context_line":"        r \u003d re.compile(\u0027[:.]\u0027)"},{"line_number":416,"context_line":"        for port in self.network.ports:"},{"line_number":417,"context_line":"            for alloc in port.fixed_ips:"},{"line_number":418,"context_line":"                hostname \u003d \u0027host-%s\u0027 % r.sub(\u0027-\u0027, alloc.ip_address)"}],"source_content_type":"text/x-python","patch_set":22,"id":"AAAAWH%2F%2Be8Y%3D","line":415,"in_reply_to":"AAAAWH%2F%2BguE%3D","updated":"2014-03-26 14:02:11.000000000","message":"Done","commit_id":"f0821712cb29f1c0c2a7f14c9a25ffcd115eacfe"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cb8a9cb8b38310407a229bc60ad9954ba54b2794","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        buf \u003d six.StringIO()"},{"line_number":439,"context_line":"        filename \u003d self.get_conf_file_name(\u0027host\u0027)"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"        LOG.debug(_(\u0027Building host file: %s\u0027), filename)"},{"line_number":442,"context_line":"        for (port, alloc, hostname, name) in self._iter_hosts():"},{"line_number":443,"context_line":"            set_tag \u003d \u0027\u0027"},{"line_number":444,"context_line":"            # (dzyu) Check if it is legal ipv6 address, if so, need wrap"}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAWH%2F%2BVXM%3D","line":441,"updated":"2014-03-27 15:14:52.000000000","message":"Nit: Slight readability boost... Replace hostname with _","commit_id":"b0fcab39554ea342310f8601f3ae1b61de17297c"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"448e971a63af64f16c94d76eca8aeef1d234b0fa","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        buf \u003d six.StringIO()"},{"line_number":439,"context_line":"        filename \u003d self.get_conf_file_name(\u0027host\u0027)"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"        LOG.debug(_(\u0027Building host file: %s\u0027), filename)"},{"line_number":442,"context_line":"        for (port, alloc, hostname, name) in self._iter_hosts():"},{"line_number":443,"context_line":"            set_tag \u003d \u0027\u0027"},{"line_number":444,"context_line":"            # (dzyu) Check if it is legal ipv6 address, if so, need wrap"}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAWH%2F%2BU80%3D","line":441,"in_reply_to":"AAAAWH%2F%2BU%2FI%3D","updated":"2014-03-27 15:53:17.000000000","message":"Understood, thanks.","commit_id":"b0fcab39554ea342310f8601f3ae1b61de17297c"},{"author":{"_account_id":8674,"name":"Yves-Gwenael Bourhis","email":"ygbourhis@gmail.com","username":"yves"},"change_message_id":"8bb32ccb2c08736b68080196c659f96aa76ade9c","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        buf \u003d six.StringIO()"},{"line_number":439,"context_line":"        filename \u003d self.get_conf_file_name(\u0027host\u0027)"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"        LOG.debug(_(\u0027Building host file: %s\u0027), filename)"},{"line_number":442,"context_line":"        for (port, alloc, hostname, name) in self._iter_hosts():"},{"line_number":443,"context_line":"            set_tag \u003d \u0027\u0027"},{"line_number":444,"context_line":"            # (dzyu) Check if it is legal ipv6 address, if so, need wrap"}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAWH%2F%2BU%2FI%3D","line":441,"in_reply_to":"AAAAWH%2F%2BVXM%3D","updated":"2014-03-27 15:51:02.000000000","message":"I can\u0027t use _ , because the _() function (gettext) is installed in Python’s builtins namespace in neutron.__init__ (this is why you do not see it imported in this file),\nand the call to this function in LOG.debug would fail.","commit_id":"b0fcab39554ea342310f8601f3ae1b61de17297c"}],"neutron/tests/unit/test_linux_dhcp.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cb8a9cb8b38310407a229bc60ad9954ba54b2794","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"                         \u002700:00:0f:aa:bb:cc,host-fdca-3ba5-a17a-4ba3--3.\u0027"},{"line_number":1041,"context_line":"                         \u0027openstacklocal,[fdca:3ba5:a17a:4ba3::3]\\n\u0027"},{"line_number":1042,"context_line":"                         \u002700:00:0f:rr:rr:rr,host-192-168-0-1.openstacklocal,\u0027"},{"line_number":1043,"context_line":"                         \u0027192.168.0.1\\n\u0027).lstrip()"},{"line_number":1044,"context_line":"        exp_addn_name \u003d \u0027/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/addn_hosts\u0027"},{"line_number":1045,"context_line":"        exp_addn_data \u003d ("},{"line_number":1046,"context_line":"            \u0027192.168.0.2\\t\u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAWH%2F%2BVcs%3D","line":1043,"updated":"2014-03-27 15:14:52.000000000","message":"I know that exp_host_data is also duplicated twice, but now the problem is worse with the duplication of exp_addn_data as well. If you could extract the data generation to a helper method which both test_reload_allocations and test_reload_allocations_stale_pid could use, that would be fantastic.","commit_id":"b0fcab39554ea342310f8601f3ae1b61de17297c"},{"author":{"_account_id":8674,"name":"Yves-Gwenael Bourhis","email":"ygbourhis@gmail.com","username":"yves"},"change_message_id":"8bb32ccb2c08736b68080196c659f96aa76ade9c","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"                         \u002700:00:0f:aa:bb:cc,host-fdca-3ba5-a17a-4ba3--3.\u0027"},{"line_number":1041,"context_line":"                         \u0027openstacklocal,[fdca:3ba5:a17a:4ba3::3]\\n\u0027"},{"line_number":1042,"context_line":"                         \u002700:00:0f:rr:rr:rr,host-192-168-0-1.openstacklocal,\u0027"},{"line_number":1043,"context_line":"                         \u0027192.168.0.1\\n\u0027).lstrip()"},{"line_number":1044,"context_line":"        exp_addn_name \u003d \u0027/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/addn_hosts\u0027"},{"line_number":1045,"context_line":"        exp_addn_data \u003d ("},{"line_number":1046,"context_line":"            \u0027192.168.0.2\\t\u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAWH%2F%2BU%2BY%3D","line":1043,"in_reply_to":"AAAAWH%2F%2BVcs%3D","updated":"2014-03-27 15:51:02.000000000","message":"Done","commit_id":"b0fcab39554ea342310f8601f3ae1b61de17297c"}]}