)]}'
{"neutron/db/l3_dvr_db.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7c56289c2b71e3307fe2c66626d6f70ff8eb599a","unresolved":false,"context_lines":[{"line_number":157,"context_line":"    def _validate_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to validate the fip agent gw port delete.\"\"\""},{"line_number":160,"context_line":"        agt_gw_flag \u003d False"},{"line_number":161,"context_line":"        port_db \u003d self._core_plugin._get_port("},{"line_number":162,"context_line":"            context, floatingip_db[\u0027fixed_port_id\u0027])"},{"line_number":163,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_2450172c","line":160,"updated":"2014-08-05 16:01:44.000000000","message":"This isn\u0027t necessary with the changes below.","commit_id":"be101bcd275dc9b2a3afebd738399de6bf90d473"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"bfe4df280d3ab51404e3cea1ed0622d32c45a9b5","unresolved":false,"context_lines":[{"line_number":157,"context_line":"    def _validate_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to validate the fip agent gw port delete.\"\"\""},{"line_number":160,"context_line":"        agt_gw_flag \u003d False"},{"line_number":161,"context_line":"        port_db \u003d self._core_plugin._get_port("},{"line_number":162,"context_line":"            context, floatingip_db[\u0027fixed_port_id\u0027])"},{"line_number":163,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_be26ef65","line":160,"in_reply_to":"baada198_2450172c","updated":"2014-08-05 17:54:18.000000000","message":"I think this is still required to initialize the state.\nThis will be over written by the output from check_fips_availability_on_host","commit_id":"be101bcd275dc9b2a3afebd738399de6bf90d473"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7c56289c2b71e3307fe2c66626d6f70ff8eb599a","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":164,"context_line":"        if fip_hostid:"},{"line_number":165,"context_line":"            agt_gw_flag \u003d self.check_fips_availability_on_host("},{"line_number":166,"context_line":"                context, fip_id, fip_hostid)"},{"line_number":167,"context_line":"        if agt_gw_flag and fip_hostid:"},{"line_number":168,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":169,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_c4583b5b","line":166,"updated":"2014-08-05 16:01:44.000000000","message":"Since you\u0027re never using the return value from self.check_fips_availability_on_host(), this can be changed to:\n\nif self.check_fips_availability_on_host(...):\n    LOG.debug(\u0027Deleting the Agent GW Port\u0027)\n    self.delete_floatingip_agent_gateway_port(context, fip_hostid)\n\nWhich removes the extra \u0027if\u0027 checks below.","commit_id":"be101bcd275dc9b2a3afebd738399de6bf90d473"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"ca34d6f698e96832e723ebaac6f1923239092227","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":164,"context_line":"        if fip_hostid:"},{"line_number":165,"context_line":"            agt_gw_flag \u003d self.check_fips_availability_on_host("},{"line_number":166,"context_line":"                context, fip_id, fip_hostid)"},{"line_number":167,"context_line":"        if agt_gw_flag and fip_hostid:"},{"line_number":168,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":169,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_b4dd2b16","line":166,"in_reply_to":"baada198_795e31ea","updated":"2014-08-05 19:59:16.000000000","message":"Yes  i got your point now.\nI have uploaded a new patch addressing your comments.","commit_id":"be101bcd275dc9b2a3afebd738399de6bf90d473"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"bfe4df280d3ab51404e3cea1ed0622d32c45a9b5","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":164,"context_line":"        if fip_hostid:"},{"line_number":165,"context_line":"            agt_gw_flag \u003d self.check_fips_availability_on_host("},{"line_number":166,"context_line":"                context, fip_id, fip_hostid)"},{"line_number":167,"context_line":"        if agt_gw_flag and fip_hostid:"},{"line_number":168,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":169,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_fe1787ed","line":166,"in_reply_to":"baada198_c4583b5b","updated":"2014-08-05 17:54:18.000000000","message":"I am still using the return value \"agt_gw_flag\" returned by  self.check_fips_availability_on_host to make the decision as to delete the fip_agent gateway port or not.\nOnly if the return value is \"True\" and if a valid \"fip_hostid\" exists I will go and delete the \"fip_agent\" gateway port.\nLet me know if you have further questions on this.","commit_id":"be101bcd275dc9b2a3afebd738399de6bf90d473"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9120d242a596d154175d3fe4cdb4ed974aa60f09","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":164,"context_line":"        if fip_hostid:"},{"line_number":165,"context_line":"            agt_gw_flag \u003d self.check_fips_availability_on_host("},{"line_number":166,"context_line":"                context, fip_id, fip_hostid)"},{"line_number":167,"context_line":"        if agt_gw_flag and fip_hostid:"},{"line_number":168,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":169,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_795e31ea","line":166,"in_reply_to":"baada198_fe1787ed","updated":"2014-08-05 18:18:11.000000000","message":"No, I meant the function could be re-written to look like this:\n\ndef _validate_fip_agent_gw_port_and_delete(\n\t\t\tself, context, floatingip_db, fip_id):\n    \"\"\"Helper function to validate the fip agent gw port and delete it.\"\"\"\n    port_db \u003d self._core_plugin._get_port(\n\t\t\tcontext, floatingip_db[\u0027fixed_port_id\u0027])\n    fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])\n    if fip_hostid and self.check_fips_availability_on_host(\n\t\t\tcontext, fip_id, fip_hostid):\n            LOG.debug(\u0027Deleting the Agent GW Port\u0027)\n            self.delete_floatingip_agent_gateway_port(context, fip_hostid)\n\nagt_gw_flag and the extra if: just go away.","commit_id":"be101bcd275dc9b2a3afebd738399de6bf90d473"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"e916125e506fd28768690feefdfa7c3db06b5926","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                              \u0027router_id\u0027: router_id,"},{"line_number":155,"context_line":"                              \u0027last_known_router_id\u0027: previous_router_id})"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def _validate_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to validate the fip agent gw port delete.\"\"\""},{"line_number":160,"context_line":"        port_db \u003d self._core_plugin._get_port("}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_09b78b89","line":157,"updated":"2014-08-13 14:50:46.000000000","message":"I think the name should be _check_fip_agent_gw_port_and_delete.  The \"validation\" is really a check that there are floatingips on a host.","commit_id":"38c36fd11a34d287fcfb0684b2b4b473bb336077"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"70517c97d212dde7cf985dee8645e723b8e79bdf","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                              \u0027router_id\u0027: router_id,"},{"line_number":155,"context_line":"                              \u0027last_known_router_id\u0027: previous_router_id})"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def _validate_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to validate the fip agent gw port delete.\"\"\""},{"line_number":160,"context_line":"        port_db \u003d self._core_plugin._get_port("}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_51a781a8","line":157,"updated":"2014-08-13 04:11:25.000000000","message":"what does this function validate? The name is a bit odd.\n\nI\u0027d rather give it a simpler name, as this only deletes the port for the agent.","commit_id":"38c36fd11a34d287fcfb0684b2b4b473bb336077"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9aad6d8ac43e2bcfbbb41089f7e1d39bcc89b70d","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                              \u0027router_id\u0027: router_id,"},{"line_number":155,"context_line":"                              \u0027last_known_router_id\u0027: previous_router_id})"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def _validate_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to validate the fip agent gw port delete.\"\"\""},{"line_number":160,"context_line":"        port_db \u003d self._core_plugin._get_port("}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_8c9d695b","line":157,"in_reply_to":"baada198_09b78b89","updated":"2014-08-13 15:00:09.000000000","message":"agreed...even though we should strive to write methods rather than poems :)\n\nIn the end, you\u0027re just deleting a port, and if it doesn\u0027t exist we just don\u0027t need to complain. If you look for examples in other software systems or even Neutron for that matter we don\u0027t have methods or functions that are called check_and_delete, but just \u0027delete\u0027. I am not saying that\u0027s Swami\u0027s fault here, but this is a practice that in my opinion leads to a software that\u0027s more verbose that it really needs to be. What\u0027s the point of being so descriptive? That\u0027s what docstrings are for.","commit_id":"38c36fd11a34d287fcfb0684b2b4b473bb336077"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"cadeeffe65b7ce6fd7a1481ae9d1ece6ac0b00f7","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                              \u0027router_id\u0027: router_id,"},{"line_number":155,"context_line":"                              \u0027last_known_router_id\u0027: previous_router_id})"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def _validate_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to validate the fip agent gw port delete.\"\"\""},{"line_number":160,"context_line":"        port_db \u003d self._core_plugin._get_port("}],"source_content_type":"text/x-python","patch_set":3,"id":"7ac7a964_f1925f9e","line":157,"in_reply_to":"baada198_8c9d695b","updated":"2014-08-14 04:07:55.000000000","message":"Will change it to \"_check_fip_agent_gw_port_and_delete\"","commit_id":"38c36fd11a34d287fcfb0684b2b4b473bb336077"},{"author":{"_account_id":9380,"name":"Sayaji Patil","email":"sayaji15@gmail.com","username":"Sayaji"},"change_message_id":"6f2c2e3b24b1e9d733d3b92a7dd42f3e22a66472","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":163,"context_line":"        if fip_hostid and self.check_fips_availability_on_host("},{"line_number":164,"context_line":"            context, fip_id, fip_hostid):"},{"line_number":165,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":166,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def delete_floatingip(self, context, id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_e78297c6","line":165,"updated":"2014-08-13 05:47:48.000000000","message":"nit: I think printing an id in the log will be more helpful","commit_id":"38c36fd11a34d287fcfb0684b2b4b473bb336077"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"e916125e506fd28768690feefdfa7c3db06b5926","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":163,"context_line":"        if fip_hostid and self.check_fips_availability_on_host("},{"line_number":164,"context_line":"            context, fip_id, fip_hostid):"},{"line_number":165,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":166,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def delete_floatingip(self, context, id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_89f2bb9a","line":165,"in_reply_to":"baada198_e78297c6","updated":"2014-08-13 14:50:46.000000000","message":"+1","commit_id":"38c36fd11a34d287fcfb0684b2b4b473bb336077"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4b54786fca1bda16ff518f5a19b9cfd52cd0899e","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        will go ahead and delete the FIP agent gateway port."},{"line_number":165,"context_line":"        If even a single VM is using the port it will not delete."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        port_db \u003d self._core_plugin._get_port("},{"line_number":168,"context_line":"            context, floatingip_db[\u0027fixed_port_id\u0027])"},{"line_number":169,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":170,"context_line":"        if fip_hostid and self.check_fips_availability_on_host("}],"source_content_type":"text/x-python","patch_set":4,"id":"7ac7a964_acbb2c71","line":167,"updated":"2014-08-14 05:37:24.000000000","message":"It could be that I\u0027m reviewing late at night but isn\u0027t this returning port_db just so that we can pass port_db[\u0027id\u0027] to the next method?  Isn\u0027t port_db[\u0027id\u0027] \u003d\u003d floatingip_db[\u0027fixed_port_id\u0027] here?  So, we already have the id, right?  No need to go fetch it.\n\nYou could alternatively pass port_db as the \u0027port\u0027 parameter to \"get_vm_port_hostid\".  This would avoid the extra call to get_port there but I think just eliminating this call would be better.\n\nI know it was this way in the original code but I think it would be appropriate to fix it here.","commit_id":"bc1741b3ce7513d54be26db9f8cf5545d668dd10"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"5da447b7d185dad1e193da285c942465c74b0580","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        will go ahead and delete the FIP agent gateway port."},{"line_number":165,"context_line":"        If even a single VM is using the port it will not delete."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        port_db \u003d self._core_plugin._get_port("},{"line_number":168,"context_line":"            context, floatingip_db[\u0027fixed_port_id\u0027])"},{"line_number":169,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid(context, port_db[\u0027id\u0027])"},{"line_number":170,"context_line":"        if fip_hostid and self.check_fips_availability_on_host("}],"source_content_type":"text/x-python","patch_set":4,"id":"1abeadc6_414a3381","line":167,"in_reply_to":"7ac7a964_acbb2c71","updated":"2014-08-19 05:15:31.000000000","message":"Makes sense, will make the necessary changes and will be upload a new patch.","commit_id":"bc1741b3ce7513d54be26db9f8cf5545d668dd10"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4b54786fca1bda16ff518f5a19b9cfd52cd0899e","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":173,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    def delete_floatingip(self, context, id):"},{"line_number":176,"context_line":"        floatingip \u003d self._get_floatingip(context, id)"},{"line_number":177,"context_line":"        if floatingip[\u0027fixed_port_id\u0027]:"},{"line_number":178,"context_line":"            admin_ctx \u003d context.elevated()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7ac7a964_eccd245f","line":175,"updated":"2014-08-14 05:37:24.000000000","message":"Just thinking out loud.  No change needed here...\n\nThe parameter id shadows the builtin \u0027id\u0027 which would be a no-no.  However, this is consistent with other definitions of the same method and should be kept as it is in this patch.","commit_id":"bc1741b3ce7513d54be26db9f8cf5545d668dd10"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"5da447b7d185dad1e193da285c942465c74b0580","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            LOG.debug(\u0027Deleting the Agent GW Port\u0027)"},{"line_number":173,"context_line":"            self.delete_floatingip_agent_gateway_port(context, fip_hostid)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    def delete_floatingip(self, context, id):"},{"line_number":176,"context_line":"        floatingip \u003d self._get_floatingip(context, id)"},{"line_number":177,"context_line":"        if floatingip[\u0027fixed_port_id\u0027]:"},{"line_number":178,"context_line":"            admin_ctx \u003d context.elevated()"}],"source_content_type":"text/x-python","patch_set":4,"id":"1abeadc6_615e4f47","line":175,"in_reply_to":"7ac7a964_eccd245f","updated":"2014-08-19 05:15:31.000000000","message":"I will Leave it as it is.","commit_id":"bc1741b3ce7513d54be26db9f8cf5545d668dd10"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"4e0c05d9ed1a22595183b62caa27b34b9de03027","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        \"\"\"Helper function to check for fip agent gw port and delete."},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        This function checks on compute nodes to make sure if there"},{"line_number":162,"context_line":"        are any VMs that are currently using the FIP agent gateway"},{"line_number":163,"context_line":"        port. If no VMs are using the FIP agent gateway port, it"},{"line_number":164,"context_line":"        will go ahead and delete the FIP agent gateway port."},{"line_number":165,"context_line":"        If even a single VM is using the port it will not delete."}],"source_content_type":"text/x-python","patch_set":6,"id":"1abeadc6_c27053a6","line":162,"updated":"2014-08-23 03:29:34.000000000","message":"rephrase: ... are any VMs using the FIP agent gateway port ...","commit_id":"d16ea9a3c284964aed4b329a71c58f2958bafd2c"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"4ddf3f109961d861c854f0d64e627768c8c7a1ad","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        \"\"\"Helper function to check for fip agent gw port and delete."},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        This function checks on compute nodes to make sure if there"},{"line_number":162,"context_line":"        are any VMs that are currently using the FIP agent gateway"},{"line_number":163,"context_line":"        port. If no VMs are using the FIP agent gateway port, it"},{"line_number":164,"context_line":"        will go ahead and delete the FIP agent gateway port."},{"line_number":165,"context_line":"        If even a single VM is using the port it will not delete."}],"source_content_type":"text/x-python","patch_set":6,"id":"1abeadc6_46942044","line":162,"in_reply_to":"1abeadc6_c27053a6","updated":"2014-08-25 20:21:24.000000000","message":"done.","commit_id":"d16ea9a3c284964aed4b329a71c58f2958bafd2c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"067984c9bd9403c7a609e4d7f3eaee749cfd533c","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        admin_ctx \u003d context.elevated()"},{"line_number":148,"context_line":"        if (not (\u0027port_id\u0027 in fip and fip[\u0027port_id\u0027])) and ("},{"line_number":149,"context_line":"            floatingip_db[\u0027fixed_port_id\u0027] is not None):"},{"line_number":150,"context_line":"                self._check_fip_agent_gw_port_and_delete("},{"line_number":151,"context_line":"                    admin_ctx, floatingip_db, fip[\u0027id\u0027])"},{"line_number":152,"context_line":"        floatingip_db.update({\u0027fixed_ip_address\u0027: internal_ip_address,"},{"line_number":153,"context_line":"                              \u0027fixed_port_id\u0027: port_id,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_66e3dc05","line":150,"updated":"2014-08-25 20:40:57.000000000","message":"this is over-indented","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"35d51aaff20e1fb191d9168d16596a00d2db6434","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        admin_ctx \u003d context.elevated()"},{"line_number":148,"context_line":"        if (not (\u0027port_id\u0027 in fip and fip[\u0027port_id\u0027])) and ("},{"line_number":149,"context_line":"            floatingip_db[\u0027fixed_port_id\u0027] is not None):"},{"line_number":150,"context_line":"                self._check_fip_agent_gw_port_and_delete("},{"line_number":151,"context_line":"                    admin_ctx, floatingip_db, fip[\u0027id\u0027])"},{"line_number":152,"context_line":"        floatingip_db.update({\u0027fixed_ip_address\u0027: internal_ip_address,"},{"line_number":153,"context_line":"                              \u0027fixed_port_id\u0027: port_id,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_2c11c3ac","line":150,"in_reply_to":"1abeadc6_66e3dc05","updated":"2014-08-25 21:30:49.000000000","message":"Good catch. Tox -e pep8 did not catch it.","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"067984c9bd9403c7a609e4d7f3eaee749cfd533c","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                              \u0027last_known_router_id\u0027: previous_router_id})"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def _check_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to check for fip agent gw port and delete."},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        This function checks on compute nodes to make sure if there"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_661cbc14","line":158,"updated":"2014-08-25 20:40:57.000000000","message":"so is this one","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"35d51aaff20e1fb191d9168d16596a00d2db6434","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                              \u0027last_known_router_id\u0027: previous_router_id})"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def _check_fip_agent_gw_port_and_delete("},{"line_number":158,"context_line":"            self, context, floatingip_db, fip_id):"},{"line_number":159,"context_line":"        \"\"\"Helper function to check for fip agent gw port and delete."},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        This function checks on compute nodes to make sure if there"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_6c22db02","line":158,"in_reply_to":"1abeadc6_661cbc14","updated":"2014-08-25 21:30:49.000000000","message":"Will fix it.","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"067984c9bd9403c7a609e4d7f3eaee749cfd533c","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        This function checks on compute nodes to make sure if there"},{"line_number":162,"context_line":"        are any VMs using the FIP agent gateway port. If no VMs are"},{"line_number":163,"context_line":"        using the FIP agent gateway port, it will go ahead and delete"},{"line_number":164,"context_line":"        the FIP agent gateway port. If even a single VM is using the"},{"line_number":165,"context_line":"        port it will not delete."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid("}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_060ce893","line":164,"updated":"2014-08-25 20:40:57.000000000","message":"what if we called this:\n\nclear_unused_agent_gw_port()?","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"35d51aaff20e1fb191d9168d16596a00d2db6434","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        This function checks on compute nodes to make sure if there"},{"line_number":162,"context_line":"        are any VMs using the FIP agent gateway port. If no VMs are"},{"line_number":163,"context_line":"        using the FIP agent gateway port, it will go ahead and delete"},{"line_number":164,"context_line":"        the FIP agent gateway port. If even a single VM is using the"},{"line_number":165,"context_line":"        port it will not delete."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid("}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_ac731314","line":164,"in_reply_to":"1abeadc6_060ce893","updated":"2014-08-25 21:30:49.000000000","message":"I think it was based on previous review comments I changed the function name to what it is now.\nBut if you like to \"clear_unused_agent_gw_port\". I will go for it.","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"c28404468646ef4527d67e36a76f173e513a3329","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        This function checks on compute nodes to make sure if there"},{"line_number":162,"context_line":"        are any VMs using the FIP agent gateway port. If no VMs are"},{"line_number":163,"context_line":"        using the FIP agent gateway port, it will go ahead and delete"},{"line_number":164,"context_line":"        the FIP agent gateway port. If even a single VM is using the"},{"line_number":165,"context_line":"        port it will not delete."},{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        fip_hostid \u003d self.get_vm_port_hostid("}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_e01fcf59","line":164,"in_reply_to":"1abeadc6_ac731314","updated":"2014-08-25 21:39:59.000000000","message":"this is just a hint here feel free to ignore it; my rule of thumb usually is to define a method that does one thing, and the name should reflect that; in this case you\u0027re effectively cleaning up a resource that\u0027s no longer needed. And to me that\u0027s one thing, but the name seems to imply it does two.","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"}],"neutron/tests/unit/db/test_l3_dvr_db.py":[{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"08ea11e57bef9d564f4f71cb3a07e8805c31d1da","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                              \u0027delete_floatingip\u0027)) as (gf, vf, df):"},{"line_number":207,"context_line":"            gf.return_value \u003d floatingip"},{"line_number":208,"context_line":"            self.mixin.delete_floatingip(self.ctx, fip_id)"},{"line_number":209,"context_line":"            self.assertTrue(vf.call_count)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_1502d108","line":209,"updated":"2014-08-08 21:34:32.000000000","message":"delete_floatingip should only call _validate_fip_agent_gw_port_and_delete once.  So it is more accurate to have this statement be:\n\n  self.assertTrue(vf.call_count\u003d\u003d1)","commit_id":"182681422c3f7f4556733680a7476fe4c0c41c13"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"f4025af54fb02d7519a729254f3e1530275367e2","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                              \u0027delete_floatingip\u0027)) as (gf, vf, df):"},{"line_number":207,"context_line":"            gf.return_value \u003d floatingip"},{"line_number":208,"context_line":"            self.mixin.delete_floatingip(self.ctx, fip_id)"},{"line_number":209,"context_line":"            self.assertTrue(vf.call_count)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_bb68d38a","line":209,"in_reply_to":"baada198_1502d108","updated":"2014-08-12 18:21:50.000000000","message":"Sure I will fix it.","commit_id":"182681422c3f7f4556733680a7476fe4c0c41c13"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"70517c97d212dde7cf985dee8645e723b8e79bdf","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                              \u0027delete_floatingip\u0027)) as (gf, vf, df):"},{"line_number":207,"context_line":"            gf.return_value \u003d floatingip"},{"line_number":208,"context_line":"            self.mixin.delete_floatingip(self.ctx, fip_id)"},{"line_number":209,"context_line":"            self.assertTrue(vf.call_count \u003d\u003d 1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_f1afb57a","line":209,"updated":"2014-08-13 04:11:25.000000000","message":"maybe assert_called_once_with([...])?","commit_id":"38c36fd11a34d287fcfb0684b2b4b473bb336077"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"067984c9bd9403c7a609e4d7f3eaee749cfd533c","unresolved":false,"context_lines":[{"line_number":10,"context_line":"# distributed under the License is distributed on an \"AS IS\" BASIS,"},{"line_number":11,"context_line":"# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or"},{"line_number":12,"context_line":"# implied."},{"line_number":13,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import contextlib"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_460b402b","line":13,"updated":"2014-08-25 20:40:57.000000000","message":"what about the coverage for the change made to:\n\n_update_fip_assoc\n\nI don\u0027t see it. It\u0027d be good to have it. Also, what about coverage of \n\n_check_fip_agent_gw_port_and_delete\n\nitself?","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"067984c9bd9403c7a609e4d7f3eaee749cfd533c","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        routers \u003d self.mixin._build_routers_list(self.ctx, routers, gw_ports)"},{"line_number":176,"context_line":"        self.assertIsNone(routers[0].get(\u0027gw_port\u0027))"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def test_delete_floatingip_without_internal_port(self):"},{"line_number":179,"context_line":"        floatingip \u003d {"},{"line_number":180,"context_line":"            \u0027id\u0027: _uuid(),"},{"line_number":181,"context_line":"            \u0027fixed_port_id\u0027: None,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_c694b0ea","line":178,"updated":"2014-08-25 20:40:57.000000000","message":"these two tests are pretty much identical, except the condition on fixed_port_id and the call_count. This can be easily turned in a util function + 2 simpler tests.","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"35d51aaff20e1fb191d9168d16596a00d2db6434","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        routers \u003d self.mixin._build_routers_list(self.ctx, routers, gw_ports)"},{"line_number":176,"context_line":"        self.assertIsNone(routers[0].get(\u0027gw_port\u0027))"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def test_delete_floatingip_without_internal_port(self):"},{"line_number":179,"context_line":"        floatingip \u003d {"},{"line_number":180,"context_line":"            \u0027id\u0027: _uuid(),"},{"line_number":181,"context_line":"            \u0027fixed_port_id\u0027: None,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_2c5f0374","line":178,"in_reply_to":"1abeadc6_c694b0ea","updated":"2014-08-25 21:30:49.000000000","message":"Will do it.","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"067984c9bd9403c7a609e4d7f3eaee749cfd533c","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                              \u0027delete_floatingip\u0027)) as (gf, vf, df):"},{"line_number":210,"context_line":"            gf.return_value \u003d floatingip"},{"line_number":211,"context_line":"            self.mixin.delete_floatingip(self.ctx, fip_id)"},{"line_number":212,"context_line":"            self.assertTrue(vf.call_count \u003d\u003d 1)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_e6f28c6e","line":212,"updated":"2014-08-25 20:40:57.000000000","message":"this can simply be self.assertTrue(vf.called) or self.assertTrue(vf.call_count)","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"35d51aaff20e1fb191d9168d16596a00d2db6434","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                              \u0027delete_floatingip\u0027)) as (gf, vf, df):"},{"line_number":210,"context_line":"            gf.return_value \u003d floatingip"},{"line_number":211,"context_line":"            self.mixin.delete_floatingip(self.ctx, fip_id)"},{"line_number":212,"context_line":"            self.assertTrue(vf.call_count \u003d\u003d 1)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1abeadc6_2cd623c6","line":212,"in_reply_to":"1abeadc6_e6f28c6e","updated":"2014-08-25 21:30:49.000000000","message":"Will do it.","commit_id":"bf7fa9b166d619f48611f6baf696d2cc0137a796"}]}