)]}'
{"neutron/agent/l3/dvr_fip_ns.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"044f1d04e5dcdcc8c0ed58cfd6439284e2c6fae6","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        # Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was"},{"line_number":138,"context_line":"        # changed to be a per-namespace attribute.  To be backwards"},{"line_number":139,"context_line":"        # compatible we need to try both if at first we fail."},{"line_number":140,"context_line":"        super(FipNamespace, self).create(ip_nonlocal_bind_value\u003d1)"},{"line_number":141,"context_line":"        try:"},{"line_number":142,"context_line":"            ip_lib.set_ip_nonlocal_bind("},{"line_number":143,"context_line":"                value\u003d1, namespace\u003dself.name, log_fail_as_error\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_7d9d2b65","line":140,"range":{"start_line":140,"start_character":41,"end_line":140,"end_character":65},"updated":"2016-11-10 10:32:21.000000000","message":"I think it\u0027s not customary to pass the argument here, instead we pass them to __init__ and then use them in parent create() as we do for ipv6 forwarding. I think we should stick to the same approach here.","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b7d44ea5c1e9f2a71d3a58039164cac893af81d4","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        # Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was"},{"line_number":138,"context_line":"        # changed to be a per-namespace attribute.  To be backwards"},{"line_number":139,"context_line":"        # compatible we need to try both if at first we fail."},{"line_number":140,"context_line":"        super(FipNamespace, self).create(ip_nonlocal_bind_value\u003d1)"},{"line_number":141,"context_line":"        try:"},{"line_number":142,"context_line":"            ip_lib.set_ip_nonlocal_bind("},{"line_number":143,"context_line":"                value\u003d1, namespace\u003dself.name, log_fail_as_error\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_2ccc6b69","line":140,"range":{"start_line":140,"start_character":8,"end_line":140,"end_character":66},"updated":"2016-11-10 09:44:39.000000000","message":"So you set it here and then again on L143? Why twice?","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2834364e4836a2568034a7f88c6425699e5ed1c1","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        # Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was"},{"line_number":138,"context_line":"        # changed to be a per-namespace attribute.  To be backwards"},{"line_number":139,"context_line":"        # compatible we need to try both if at first we fail."},{"line_number":140,"context_line":"        super(FipNamespace, self).create(ip_nonlocal_bind_value\u003d1)"},{"line_number":141,"context_line":"        try:"},{"line_number":142,"context_line":"            ip_lib.set_ip_nonlocal_bind("},{"line_number":143,"context_line":"                value\u003d1, namespace\u003dself.name, log_fail_as_error\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_ec099385","line":140,"range":{"start_line":140,"start_character":8,"end_line":140,"end_character":66},"in_reply_to":"9a629dbe_2ccc6b69","updated":"2016-11-10 09:47:37.000000000","message":"Sorry, the block of line 141 should be deleted.","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"044f1d04e5dcdcc8c0ed58cfd6439284e2c6fae6","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        # Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was"},{"line_number":138,"context_line":"        # changed to be a per-namespace attribute.  To be backwards"},{"line_number":139,"context_line":"        # compatible we need to try both if at first we fail."},{"line_number":140,"context_line":"        super(FipNamespace, self).create(ip_nonlocal_bind_value\u003d1)"},{"line_number":141,"context_line":"        try:"},{"line_number":142,"context_line":"            ip_lib.set_ip_nonlocal_bind("},{"line_number":143,"context_line":"                value\u003d1, namespace\u003dself.name, log_fail_as_error\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_bdd42320","line":140,"range":{"start_line":140,"start_character":8,"end_line":140,"end_character":66},"in_reply_to":"9a629dbe_ec099385","updated":"2016-11-10 10:32:21.000000000","message":"Note that we still want to call for root namespace, as in line 148.","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"}],"neutron/agent/l3/namespaces.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b7d44ea5c1e9f2a71d3a58039164cac893af81d4","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.driver \u003d driver"},{"line_number":90,"context_line":"        self.use_ipv6 \u003d use_ipv6"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def create(self, ip_nonlocal_bind_value\u003d0):"},{"line_number":93,"context_line":"        \"\"\"Create a new namespace."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        After the namespace is created, the following kernel options are put"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_ac05db7a","line":92,"range":{"start_line":92,"start_character":21,"end_line":92,"end_character":43},"updated":"2016-11-10 09:44:39.000000000","message":"I don\u0027t like that we set 3 knobs while one is configurable. I think it\u0027s better how it was before where you have a dedicated namespace and kernel option per namespace is configured based on which namespace class you use.","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"044f1d04e5dcdcc8c0ed58cfd6439284e2c6fae6","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.driver \u003d driver"},{"line_number":90,"context_line":"        self.use_ipv6 \u003d use_ipv6"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def create(self, ip_nonlocal_bind_value\u003d0):"},{"line_number":93,"context_line":"        \"\"\"Create a new namespace."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        After the namespace is created, the following kernel options are put"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_7dc28b35","line":92,"range":{"start_line":92,"start_character":21,"end_line":92,"end_character":43},"in_reply_to":"9a629dbe_1dcd0fb7","updated":"2016-11-10 10:32:21.000000000","message":"I don\u0027t think replacing an attribute is the best we can do. Ideally, we would not even need create()s in subclasses, leaving the parent to behave depending on arguments we pass to __init__ (like we do for use_ipv6).","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f84159802d64c9261333c5784b86ebfca39d83c3","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.driver \u003d driver"},{"line_number":90,"context_line":"        self.use_ipv6 \u003d use_ipv6"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def create(self, ip_nonlocal_bind_value\u003d0):"},{"line_number":93,"context_line":"        \"\"\"Create a new namespace."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        After the namespace is created, the following kernel options are put"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_d2ef061c","line":92,"range":{"start_line":92,"start_character":21,"end_line":92,"end_character":43},"in_reply_to":"9a629dbe_6ce3c3a4","updated":"2016-11-10 10:08:42.000000000","message":"So what is the issue? You have a subclass that replaces an attribute of superclass with more specific object and throws the original attribute away. I don\u0027t see anything wrong with that.","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2834364e4836a2568034a7f88c6425699e5ed1c1","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.driver \u003d driver"},{"line_number":90,"context_line":"        self.use_ipv6 \u003d use_ipv6"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def create(self, ip_nonlocal_bind_value\u003d0):"},{"line_number":93,"context_line":"        \"\"\"Create a new namespace."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        After the namespace is created, the following kernel options are put"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_6ce3c3a4","line":92,"range":{"start_line":92,"start_character":21,"end_line":92,"end_character":43},"in_reply_to":"9a629dbe_ac05db7a","updated":"2016-11-10 09:47:37.000000000","message":"The kernel option is still configured based on which namespace class you use, it just defaults to 0.\n\nThe issue with before is that it messes with initialisation of objects, creating 2 Namespace objects at times and discarding the first since it\u0027s \"not the HA namespace we need\".","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5c31b635df38e70f5f2c77b86e6f0a031e29494e","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.driver \u003d driver"},{"line_number":90,"context_line":"        self.use_ipv6 \u003d use_ipv6"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def create(self, ip_nonlocal_bind_value\u003d0):"},{"line_number":93,"context_line":"        \"\"\"Create a new namespace."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        After the namespace is created, the following kernel options are put"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_1dcd0fb7","line":92,"range":{"start_line":92,"start_character":21,"end_line":92,"end_character":43},"in_reply_to":"9a629dbe_d2d9c658","updated":"2016-11-10 10:26:35.000000000","message":"Having system interaction in __init__ is an anti-pattern exactly for this reason. That\u0027s why we have create() methods to avoid such side-effects.","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a960cf78fa9453a2350e714ac450811d4e309362","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.driver \u003d driver"},{"line_number":90,"context_line":"        self.use_ipv6 \u003d use_ipv6"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def create(self, ip_nonlocal_bind_value\u003d0):"},{"line_number":93,"context_line":"        \"\"\"Create a new namespace."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        After the namespace is created, the following kernel options are put"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_d2d9c658","line":92,"range":{"start_line":92,"start_character":21,"end_line":92,"end_character":43},"in_reply_to":"9a629dbe_d2ef061c","updated":"2016-11-10 10:10:47.000000000","message":"The issue is that if we get to the point where 2 different classes (in the same hierarchy) create a Namespace object, something is wrong. Imagine someone in the future changes the Namespace.__init__ to actually interact with the operating system somehow - this specific case will cause 2 different interactions where only one is intended.","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"044f1d04e5dcdcc8c0ed58cfd6439284e2c6fae6","unresolved":false,"context_lines":[{"line_number":106,"context_line":"            cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv6.conf.all.forwarding\u003d1\u0027]"},{"line_number":107,"context_line":"            ip_wrapper.netns.execute(cmd)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        ip_lib.set_ip_nonlocal_bind(value\u003dvalue, namespace\u003dself)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def delete(self):"},{"line_number":112,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_dde397d7","line":109,"range":{"start_line":109,"start_character":42,"end_line":109,"end_character":47},"updated":"2016-11-10 10:32:21.000000000","message":"you mean ip_nonlocal_bind_value?","commit_id":"33887fd8c9bcc49007297dbe75a7c0df6d447422"}],"neutron/agent/linux/ip_lib.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"044f1d04e5dcdcc8c0ed58cfd6439284e2c6fae6","unresolved":false,"context_lines":[{"line_number":1082,"context_line":"        set_ip_nonlocal_bind(value\u003d0, namespace\u003dnamespace)"},{"line_number":1083,"context_line":"    except RuntimeError as rte:"},{"line_number":1084,"context_line":"        LOG.warning("},{"line_number":1085,"context_line":"            _LW(\"Setting %(knob)s\u003d0 in namespace %(ns)s failed: %(err)s It \""},{"line_number":1086,"context_line":"                \"is not set to 0 in root namespace in order to not break \""},{"line_number":1087,"context_line":"                \"DVR which requires this knob to be set to 1. This may \""},{"line_number":1088,"context_line":"                \"introduce a race between moving floating IP to a different \""},{"line_number":1089,"context_line":"                \"network node and peer sides getting populated ARP cache for \""},{"line_number":1090,"context_line":"                \"given floating IP.\"),"},{"line_number":1091,"context_line":"            {\u0027knob\u0027: IP_NONLOCAL_BIND,"},{"line_number":1092,"context_line":"             \u0027ns\u0027: namespace,"},{"line_number":1093,"context_line":"             \u0027err\u0027: rte})"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_bd2c2303","side":"PARENT","line":1090,"range":{"start_line":1085,"start_character":64,"end_line":1090,"end_character":35},"updated":"2016-11-10 10:32:21.000000000","message":"so now you loose that valuable information?","commit_id":"ed41be601d35f3c729c6e1e948efc7a02be24544"}]}