)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Roee Agiman \u003cragiman@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-11-29 11:37:14 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add scenario test that associate security group to port and spawning VM"},{"line_number":8,"context_line":"using that port."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"This test is creating two subnets on the same network, creating ports"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_74f22672","line":7,"updated":"2017-11-29 19:22:02.000000000","message":"title should be short, and not span into two lines, f.e. \u0027scenario: spawn instance with port associated to SG\u0027","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2017-11-29 11:37:14 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add scenario test that associate security group to port and spawning VM"},{"line_number":8,"context_line":"using that port."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"This test is creating two subnets on the same network, creating ports"},{"line_number":11,"context_line":"with a pre-made security group, and then spawning two VMs and assigning"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_34e81e7a","line":8,"range":{"start_line":8,"start_character":15,"end_line":8,"end_character":16},"updated":"2017-11-29 19:22:02.000000000","message":"no trailing dot needed","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add scenario test that associate security group to port and spawning VM"},{"line_number":8,"context_line":"using that port."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"This test is creating two subnets on the same network, creating ports"},{"line_number":11,"context_line":"with a pre-made security group, and then spawning two VMs and assigning"},{"line_number":12,"context_line":"them with the group created - to verify that it is inherited and"},{"line_number":13,"context_line":"enforced."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_14015a38","line":10,"range":{"start_line":10,"start_character":10,"end_line":10,"end_character":23},"updated":"2017-11-29 19:22:02.000000000","message":"-\u003e creates","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add scenario test that associate security group to port and spawning VM"},{"line_number":8,"context_line":"using that port."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"This test is creating two subnets on the same network, creating ports"},{"line_number":11,"context_line":"with a pre-made security group, and then spawning two VMs and assigning"},{"line_number":12,"context_line":"them with the group created - to verify that it is inherited and"},{"line_number":13,"context_line":"enforced."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_b4fbce44","line":10,"range":{"start_line":10,"start_character":55,"end_line":10,"end_character":64},"updated":"2017-11-29 19:22:02.000000000","message":"creates two subnets, ... ports ..., and then spawns...","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":8,"context_line":"using that port."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"This test is creating two subnets on the same network, creating ports"},{"line_number":11,"context_line":"with a pre-made security group, and then spawning two VMs and assigning"},{"line_number":12,"context_line":"them with the group created - to verify that it is inherited and"},{"line_number":13,"context_line":"enforced."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_349d7e09","line":11,"range":{"start_line":11,"start_character":54,"end_line":11,"end_character":57},"updated":"2017-11-29 19:22:02.000000000","message":"it\u0027s usually \u0027instance\u0027 in openstack-speak (it may as well be a nova managed container or bare metal node).","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":8,"context_line":"using that port."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"This test is creating two subnets on the same network, creating ports"},{"line_number":11,"context_line":"with a pre-made security group, and then spawning two VMs and assigning"},{"line_number":12,"context_line":"them with the group created - to verify that it is inherited and"},{"line_number":13,"context_line":"enforced."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I40f8b0cac360594a65fb9f6023930af3322cac58"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_34b6de8b","line":12,"range":{"start_line":11,"start_character":58,"end_line":12,"end_character":28},"updated":"2017-11-29 19:22:02.000000000","message":"but you don\u0027t assign instances to SG, you assign ports to SG. And that\u0027s already covered earlier in the sentence. Consider removal?","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"}],"neutron_tempest_plugin/scenario/test_security_groups.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"afaa4f0adb4e242cd1c1034b8252ee25d55bf72a","unresolved":false,"context_lines":[{"line_number":26,"context_line":"CONF \u003d config.CONF"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class NetworkDefaultSecGroupTest(base.BaseTempestTestCase):"},{"line_number":30,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":31,"context_line":"    required_extensions \u003d [\u0027router\u0027, \u0027security-group\u0027]"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_b748b05b","line":29,"range":{"start_line":29,"start_character":6,"end_line":29,"end_character":32},"updated":"2017-11-29 19:24:38.000000000","message":"I am also concerned about the name of the test class. It suggests that it is for test cases that validate behavior of \u0027default\u0027 security group, but we seem to use it as a general \u0027catch all\u0027 class for SG related test cases. Since your scenario doesn\u0027t target default security group, please create a new class with distinct name for it. We may then also move test cases that are already in the class but don\u0027t target default SG to the new class later in a separate followup patch.","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"909ada1bc49934b41456dc372a41206f315c02ae","unresolved":false,"context_lines":[{"line_number":26,"context_line":"CONF \u003d config.CONF"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class NetworkDefaultSecGroupTest(base.BaseTempestTestCase):"},{"line_number":30,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":31,"context_line":"    required_extensions \u003d [\u0027router\u0027, \u0027security-group\u0027]"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_65bdf000","line":29,"range":{"start_line":29,"start_character":6,"end_line":29,"end_character":32},"in_reply_to":"df87a7cf_8d2bd292","updated":"2017-12-21 12:15:33.000000000","message":"Done","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"49fe3c8766c9744b2fed696d954848335fdaf42f","unresolved":false,"context_lines":[{"line_number":26,"context_line":"CONF \u003d config.CONF"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class NetworkDefaultSecGroupTest(base.BaseTempestTestCase):"},{"line_number":30,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":31,"context_line":"    required_extensions \u003d [\u0027router\u0027, \u0027security-group\u0027]"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_8d2bd292","line":29,"range":{"start_line":29,"start_character":6,"end_line":29,"end_character":32},"in_reply_to":"df87a7cf_f48594ae","updated":"2017-12-04 22:22:21.000000000","message":"Your patch could introduce a new class. Then a follow up may move some of existing test cases into that new class. But nothing blocks us from doing the right thing for the test case you add with this patch.\n\nAlternatively, you just rename this class into something more generic that would cover both your test case as well as existing ones. There is no huge reason to have a dedicated class just for default security group validation (yet, at least until there are a lot more test cases in the class).","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"b1845275e697486bae5a8df06cf9e4afd9eb26f0","unresolved":false,"context_lines":[{"line_number":26,"context_line":"CONF \u003d config.CONF"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class NetworkDefaultSecGroupTest(base.BaseTempestTestCase):"},{"line_number":30,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":31,"context_line":"    required_extensions \u003d [\u0027router\u0027, \u0027security-group\u0027]"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_f48594ae","line":29,"range":{"start_line":29,"start_character":6,"end_line":29,"end_character":32},"in_reply_to":"ff82abbf_b748b05b","updated":"2017-12-04 16:29:51.000000000","message":"Not really sure how to fix this, I mean, this is a comment that relevant to many of the cases within this file, maybe we\u0027ll propose another patch with proper arrangement - more like a decorative patch?","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":53,"context_line":"            }"},{"line_number":54,"context_line":"            if ports is not None:"},{"line_number":55,"context_line":"                server_args[\u0027networks\u0027] \u003d [{\u0027uuid\u0027: self.network[\u0027id\u0027],"},{"line_number":56,"context_line":"                                            \u0027port\u0027: ports[i][\u0027id\u0027]}]"},{"line_number":57,"context_line":"            servers.append(self.create_server(**server_args))"},{"line_number":58,"context_line":"        for i, server in enumerate(servers):"},{"line_number":59,"context_line":"            waiters.wait_for_server_status("}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_d479b2c4","line":56,"range":{"start_line":56,"start_character":58,"end_line":56,"end_character":59},"updated":"2017-11-29 19:22:02.000000000","message":"(nitty nit hypothetical) and what if ports is an empty list? would it make sense to fail with KeyError here then? Should we just do nothing instead?","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ba63857f54db6741fa9596657155e9458b762393","unresolved":false,"context_lines":[{"line_number":53,"context_line":"            }"},{"line_number":54,"context_line":"            if ports is not None:"},{"line_number":55,"context_line":"                server_args[\u0027networks\u0027] \u003d [{\u0027uuid\u0027: self.network[\u0027id\u0027],"},{"line_number":56,"context_line":"                                            \u0027port\u0027: ports[i][\u0027id\u0027]}]"},{"line_number":57,"context_line":"            servers.append(self.create_server(**server_args))"},{"line_number":58,"context_line":"        for i, server in enumerate(servers):"},{"line_number":59,"context_line":"            waiters.wait_for_server_status("}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_501d8e0d","line":56,"range":{"start_line":56,"start_character":58,"end_line":56,"end_character":59},"in_reply_to":"ff82abbf_c2dfa2d4","updated":"2017-11-30 19:21:01.000000000","message":"True","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"b50ade5edb6c7918cc7c344811c9f28f1f9904e1","unresolved":false,"context_lines":[{"line_number":53,"context_line":"            }"},{"line_number":54,"context_line":"            if ports is not None:"},{"line_number":55,"context_line":"                server_args[\u0027networks\u0027] \u003d [{\u0027uuid\u0027: self.network[\u0027id\u0027],"},{"line_number":56,"context_line":"                                            \u0027port\u0027: ports[i][\u0027id\u0027]}]"},{"line_number":57,"context_line":"            servers.append(self.create_server(**server_args))"},{"line_number":58,"context_line":"        for i, server in enumerate(servers):"},{"line_number":59,"context_line":"            waiters.wait_for_server_status("}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_c2dfa2d4","line":56,"range":{"start_line":56,"start_character":58,"end_line":56,"end_character":59},"in_reply_to":"ff82abbf_d479b2c4","updated":"2017-11-30 13:20:18.000000000","message":"ports kwargs is expecting a list, even if only one element is in that list.\nempty list will fail, but if ports are not needed, why would you pass an empty list?\nThe gates should fail in this case.","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        self._test_ip_prefix(rule_list, should_succeed\u003dFalse)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    @decorators.idempotent_id(\u0027f07d0159-8f9e-4faa-87f5-a869ab0ad488\u0027)"},{"line_number":221,"context_line":"    def test_multiple_subnets(self):"},{"line_number":222,"context_line":"        # create another subnet on the network created in setup"},{"line_number":223,"context_line":"        self.create_subnet(self.network)"},{"line_number":224,"context_line":"        self.create_router_interface(self.create_router_by_client()[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_d4521235","line":221,"range":{"start_line":221,"start_character":13,"end_line":221,"end_character":29},"updated":"2017-11-29 19:22:02.000000000","message":"wait, why are subnets involved? it doesn\u0027t seem to be covered in the commit message. please explain (in the commit message), or fix the test case name / body.","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"b1845275e697486bae5a8df06cf9e4afd9eb26f0","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        self._test_ip_prefix(rule_list, should_succeed\u003dFalse)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    @decorators.idempotent_id(\u0027f07d0159-8f9e-4faa-87f5-a869ab0ad488\u0027)"},{"line_number":221,"context_line":"    def test_multiple_subnets(self):"},{"line_number":222,"context_line":"        # create another subnet on the network created in setup"},{"line_number":223,"context_line":"        self.create_subnet(self.network)"},{"line_number":224,"context_line":"        self.create_router_interface(self.create_router_by_client()[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"df87a7cf_94abe81b","line":221,"range":{"start_line":221,"start_character":13,"end_line":221,"end_character":29},"in_reply_to":"ff82abbf_d4521235","updated":"2017-12-04 16:29:51.000000000","message":"This was a scenario that used to failed in the past, that is the answer that I got from the original test creator - I\u0027ll update the commit message.","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":230,"context_line":"                 self.create_pingable_secgroup_rule]"},{"line_number":231,"context_line":"        for func in funcs:"},{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create a port in two different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for i, subnet in enumerate(self.subnets):"},{"line_number":236,"context_line":"            ports.append(self.create_port("}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_34125e63","line":233,"range":{"start_line":233,"start_character":9,"end_line":233,"end_character":48},"updated":"2017-11-29 19:22:02.000000000","message":"nit: ports are not in subnets, they are in networks. what you probably mean here is \u0027create ports with fixed IPs from different subnets\u0027. Though again, I am not sure using two separate subnets is supposed to be part of this scenario. It shouldn\u0027t influence anything, whether you use same IP range or two different ranges.","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"b50ade5edb6c7918cc7c344811c9f28f1f9904e1","unresolved":false,"context_lines":[{"line_number":230,"context_line":"                 self.create_pingable_secgroup_rule]"},{"line_number":231,"context_line":"        for func in funcs:"},{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create a port in two different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for i, subnet in enumerate(self.subnets):"},{"line_number":236,"context_line":"            ports.append(self.create_port("}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_d1176e45","line":233,"range":{"start_line":233,"start_character":9,"end_line":233,"end_character":48},"in_reply_to":"ff82abbf_34125e63","updated":"2017-11-30 13:20:18.000000000","message":"Done","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"539825af327156af28357a2f398ae3fad0ae42d1","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create a port in two different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for i, subnet in enumerate(self.subnets):"},{"line_number":236,"context_line":"            ports.append(self.create_port("},{"line_number":237,"context_line":"                self.network, fixed_ips\u003d[{\u0027subnet_id\u0027: subnet[\u0027id\u0027]}],"},{"line_number":238,"context_line":"                security_groups\u003d[secgrp[\u0027security_group\u0027][\u0027id\u0027]]))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_5415a247","line":235,"range":{"start_line":235,"start_character":12,"end_line":235,"end_character":14},"updated":"2017-11-29 19:22:02.000000000","message":"it doesn\u0027t seems like you use \u0027i\u0027 anywhere in the for-body, so maybe instead just iterate through self.subnets as in:\n\nfor subnet in self.subnets:\n    ...","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"b50ade5edb6c7918cc7c344811c9f28f1f9904e1","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create a port in two different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for i, subnet in enumerate(self.subnets):"},{"line_number":236,"context_line":"            ports.append(self.create_port("},{"line_number":237,"context_line":"                self.network, fixed_ips\u003d[{\u0027subnet_id\u0027: subnet[\u0027id\u0027]}],"},{"line_number":238,"context_line":"                security_groups\u003d[secgrp[\u0027security_group\u0027][\u0027id\u0027]]))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_42edd289","line":235,"range":{"start_line":235,"start_character":12,"end_line":235,"end_character":14},"in_reply_to":"ff82abbf_5415a247","updated":"2017-11-30 13:20:18.000000000","message":"Done","commit_id":"c3923a6b22d28eeb729b37438002d049f4c5a8c3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b1fec33e810dd3a720c94c8c9ad9ab12f3f6a88b","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def create_vm_testing_sec_grp(self, num_servers\u003d2, security_groups\u003dNone,"},{"line_number":44,"context_line":"                                  ports\u003dNone):"},{"line_number":45,"context_line":"        servers, fips, server_ssh_clients \u003d ([], [], [])"},{"line_number":46,"context_line":"        for i in range(num_servers):"},{"line_number":47,"context_line":"            server_args \u003d {"},{"line_number":48,"context_line":"                \u0027flavor_ref\u0027: CONF.compute.flavor_ref,"},{"line_number":49,"context_line":"                \u0027image_ref\u0027: CONF.compute.image_ref,"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_2ae94c19","line":46,"updated":"2017-12-04 20:57:33.000000000","message":"shouldn\u0027t \u0027num_servers\u0027 be taken from \u0027ports\u0027 lenght if \u0027ports\u0027 is not None? If You will have e.g. less ports given then num_servers than it will raise IndexError in L56","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"49fe3c8766c9744b2fed696d954848335fdaf42f","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def create_vm_testing_sec_grp(self, num_servers\u003d2, security_groups\u003dNone,"},{"line_number":44,"context_line":"                                  ports\u003dNone):"},{"line_number":45,"context_line":"        servers, fips, server_ssh_clients \u003d ([], [], [])"},{"line_number":46,"context_line":"        for i in range(num_servers):"},{"line_number":47,"context_line":"            server_args \u003d {"},{"line_number":48,"context_line":"                \u0027flavor_ref\u0027: CONF.compute.flavor_ref,"},{"line_number":49,"context_line":"                \u0027image_ref\u0027: CONF.compute.image_ref,"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_482db893","line":46,"in_reply_to":"df87a7cf_2ae94c19","updated":"2017-12-04 22:22:21.000000000","message":"Roee had a valid counter-argument in a previous patchset suggesting that if caller passes incompatible argument values (e.g. num_servers \u003d 2 and len(ports) \u003d\u003d 3) then it makes sense to blow up in caller\u0027s face.","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"211341c01f2c597e477a5fb57ce852686662744d","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def create_vm_testing_sec_grp(self, num_servers\u003d2, security_groups\u003dNone,"},{"line_number":44,"context_line":"                                  ports\u003dNone):"},{"line_number":45,"context_line":"        servers, fips, server_ssh_clients \u003d ([], [], [])"},{"line_number":46,"context_line":"        for i in range(num_servers):"},{"line_number":47,"context_line":"            server_args \u003d {"},{"line_number":48,"context_line":"                \u0027flavor_ref\u0027: CONF.compute.flavor_ref,"},{"line_number":49,"context_line":"                \u0027image_ref\u0027: CONF.compute.image_ref,"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf8cb3f7_6d00f035","line":46,"in_reply_to":"df87a7cf_482db893","updated":"2017-12-21 13:12:10.000000000","message":"ok, that sounds reasonable for me :)","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b1fec33e810dd3a720c94c8c9ad9ab12f3f6a88b","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        funcs \u003d [self.create_loginable_secgroup_rule,"},{"line_number":230,"context_line":"                 self.create_pingable_secgroup_rule]"},{"line_number":231,"context_line":"        for func in funcs:"},{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create ports with fixed IPs from different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for subnet in self.subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_4aee1023","line":232,"updated":"2017-12-04 20:57:33.000000000","message":"maybe it is not very important but IMHO it\u0027s too complicated for simple thing. You just have two methods to call and it would be easier to just call them one by one instead of creating such \"funcs\" list and iterate over its elements","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"909ada1bc49934b41456dc372a41206f315c02ae","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        funcs \u003d [self.create_loginable_secgroup_rule,"},{"line_number":230,"context_line":"                 self.create_pingable_secgroup_rule]"},{"line_number":231,"context_line":"        for func in funcs:"},{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create ports with fixed IPs from different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for subnet in self.subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_65b65041","line":232,"in_reply_to":"df87a7cf_4aee1023","updated":"2017-12-21 12:15:33.000000000","message":"Done","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"49fe3c8766c9744b2fed696d954848335fdaf42f","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        funcs \u003d [self.create_loginable_secgroup_rule,"},{"line_number":230,"context_line":"                 self.create_pingable_secgroup_rule]"},{"line_number":231,"context_line":"        for func in funcs:"},{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create ports with fixed IPs from different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for subnet in self.subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_a83b24cf","line":232,"in_reply_to":"df87a7cf_4aee1023","updated":"2017-12-04 22:22:21.000000000","message":"Yes, agreed. Let\u0027s just have two lines. If the concern here is that you need to extract id twice, then just extract it in a very first line:\n\nid \u003d ...\nself.create...rule(id)\nself.create...rule(id)","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"211341c01f2c597e477a5fb57ce852686662744d","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        funcs \u003d [self.create_loginable_secgroup_rule,"},{"line_number":230,"context_line":"                 self.create_pingable_secgroup_rule]"},{"line_number":231,"context_line":"        for func in funcs:"},{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create ports with fixed IPs from different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for subnet in self.subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf8cb3f7_0dfda439","line":232,"in_reply_to":"df87a7cf_a5b0d844","updated":"2017-12-21 13:12:10.000000000","message":"thx :)","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"909ada1bc49934b41456dc372a41206f315c02ae","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        funcs \u003d [self.create_loginable_secgroup_rule,"},{"line_number":230,"context_line":"                 self.create_pingable_secgroup_rule]"},{"line_number":231,"context_line":"        for func in funcs:"},{"line_number":232,"context_line":"            func(secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":233,"context_line":"        # create ports with fixed IPs from different subnets"},{"line_number":234,"context_line":"        ports \u003d []"},{"line_number":235,"context_line":"        for subnet in self.subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_a5b0d844","line":232,"in_reply_to":"df87a7cf_a83b24cf","updated":"2017-12-21 12:15:33.000000000","message":"Done","commit_id":"4d5bdc163ed3c2b6d4f58c6526a7de99daa9144e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"211341c01f2c597e477a5fb57ce852686662744d","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":229,"context_line":"        # create two ports with fixed IPs and the security group created"},{"line_number":230,"context_line":"        ports \u003d []"},{"line_number":231,"context_line":"        for i in range(2):"},{"line_number":232,"context_line":"            ports.append(self.create_port("},{"line_number":233,"context_line":"                self.network, fixed_ips\u003d[{\u0027subnet_id\u0027: self.subnets[0][\u0027id\u0027]}],"},{"line_number":234,"context_line":"                security_groups\u003d[secgrp[\u0027security_group\u0027][\u0027id\u0027]]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf8cb3f7_a8a906e0","line":231,"updated":"2017-12-21 13:12:10.000000000","message":"sorry if my question is stupid but I just don\u0027t understand one thing (maybe I\u0027m missing something).\nWhy You need to spawn 2 instances with this security group? Shouldn\u0027t 1 be enough?\nAs I understand from code, at the end You are testing connectivity to both instances from test node, You are not testing connection between instances, right?","commit_id":"8ebadb3702fce4382eb6fb4c1af10556c3648676"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"a23570535dcbec91d2266f8f67730fe7451c374b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":229,"context_line":"        # create two ports with fixed IPs and the security group created"},{"line_number":230,"context_line":"        ports \u003d []"},{"line_number":231,"context_line":"        for i in range(2):"},{"line_number":232,"context_line":"            ports.append(self.create_port("},{"line_number":233,"context_line":"                self.network, fixed_ips\u003d[{\u0027subnet_id\u0027: self.subnets[0][\u0027id\u0027]}],"},{"line_number":234,"context_line":"                security_groups\u003d[secgrp[\u0027security_group\u0027][\u0027id\u0027]]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf8cb3f7_e8a5ceb6","line":231,"in_reply_to":"bf8cb3f7_a8458619","updated":"2017-12-21 13:26:40.000000000","message":"ok, so in the past such test was failing with two or more ports (instances) and was fine with one port (instance), right?\nAnd now You want to cover by test this what was failing in the past, right?\nIf so, maybe short comment at top of this test method would be useful for someone who will read this code in the future? What You think?","commit_id":"8ebadb3702fce4382eb6fb4c1af10556c3648676"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"c9d18c50a16e3a4b88312bc086835b7385ff9588","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":229,"context_line":"        # create two ports with fixed IPs and the security group created"},{"line_number":230,"context_line":"        ports \u003d []"},{"line_number":231,"context_line":"        for i in range(2):"},{"line_number":232,"context_line":"            ports.append(self.create_port("},{"line_number":233,"context_line":"                self.network, fixed_ips\u003d[{\u0027subnet_id\u0027: self.subnets[0][\u0027id\u0027]}],"},{"line_number":234,"context_line":"                security_groups\u003d[secgrp[\u0027security_group\u0027][\u0027id\u0027]]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf8cb3f7_a8458619","line":231,"in_reply_to":"bf8cb3f7_a8a906e0","updated":"2017-12-21 13:17:14.000000000","message":"Your question is excellent, and has been my question for a while :)\nThere has been an issue, way back in the days, that this scenario with two ports was failing.\nThis is covering the matter, but I can\u0027t find any documentation from back-in-the-days.\n\nFunctionality-wise, testing one port will cover everything.","commit_id":"8ebadb3702fce4382eb6fb4c1af10556c3648676"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"dd9397ee22df1c45a2105071806840dd8064571b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            secgroup_id\u003dsecgrp[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":229,"context_line":"        # create two ports with fixed IPs and the security group created"},{"line_number":230,"context_line":"        ports \u003d []"},{"line_number":231,"context_line":"        for i in range(2):"},{"line_number":232,"context_line":"            ports.append(self.create_port("},{"line_number":233,"context_line":"                self.network, fixed_ips\u003d[{\u0027subnet_id\u0027: self.subnets[0][\u0027id\u0027]}],"},{"line_number":234,"context_line":"                security_groups\u003d[secgrp[\u0027security_group\u0027][\u0027id\u0027]]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf8cb3f7_033573da","line":231,"in_reply_to":"bf8cb3f7_e8a5ceb6","updated":"2017-12-21 13:54:22.000000000","message":"Absolutely right. I\u0027ll add proper comments.","commit_id":"8ebadb3702fce4382eb6fb4c1af10556c3648676"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"211341c01f2c597e477a5fb57ce852686662744d","unresolved":false,"context_lines":[{"line_number":240,"context_line":"            self.ping_ip_address(fip[\u0027floating_ip_address\u0027])"},{"line_number":241,"context_line":"            self.check_connectivity(fip[\u0027floating_ip_address\u0027],"},{"line_number":242,"context_line":"                                    CONF.validation.image_ssh_user,"},{"line_number":243,"context_line":"                                        self.keypair[\u0027private_key\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf8cb3f7_0d9284f7","line":243,"range":{"start_line":243,"start_character":36,"end_line":243,"end_character":40},"updated":"2017-12-21 13:12:10.000000000","message":"nit: is this indentation fine?","commit_id":"8ebadb3702fce4382eb6fb4c1af10556c3648676"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"c9d18c50a16e3a4b88312bc086835b7385ff9588","unresolved":false,"context_lines":[{"line_number":240,"context_line":"            self.ping_ip_address(fip[\u0027floating_ip_address\u0027])"},{"line_number":241,"context_line":"            self.check_connectivity(fip[\u0027floating_ip_address\u0027],"},{"line_number":242,"context_line":"                                    CONF.validation.image_ssh_user,"},{"line_number":243,"context_line":"                                        self.keypair[\u0027private_key\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf8cb3f7_0891129a","line":243,"range":{"start_line":243,"start_character":36,"end_line":243,"end_character":40},"in_reply_to":"bf8cb3f7_0d9284f7","updated":"2017-12-21 13:17:14.000000000","message":":O\nit\u0027s not","commit_id":"8ebadb3702fce4382eb6fb4c1af10556c3648676"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e30e621ad8fae8a5ed3ae0390439b94241d8cef","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        cls.keypair \u003d cls.create_keypair()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def create_vm_testing_sec_grp(self, num_servers\u003d2, security_groups\u003dNone,"},{"line_number":44,"context_line":"                                  ports\u003dNone):"},{"line_number":45,"context_line":"        servers, fips, server_ssh_clients \u003d ([], [], [])"},{"line_number":46,"context_line":"        for i in range(num_servers):"},{"line_number":47,"context_line":"            server_args \u003d {"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f91af0f_892194b7","line":44,"updated":"2018-01-02 20:10:29.000000000","message":"This is a little confusing since port must be a list, and there must be the same number as num_servers.  If it was agent code I\u0027d say to add a check, but being a test it will just blow-up and fail, but maybe add a comment so it\u0027s obvious.","commit_id":"a30091183ccb065010b4deea54074ef15c5d2106"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"bc6e1ac0595eb9193e9347318b86faca1c953883","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        cls.keypair \u003d cls.create_keypair()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def create_vm_testing_sec_grp(self, num_servers\u003d2, security_groups\u003dNone,"},{"line_number":44,"context_line":"                                  ports\u003dNone):"},{"line_number":45,"context_line":"        servers, fips, server_ssh_clients \u003d ([], [], [])"},{"line_number":46,"context_line":"        for i in range(num_servers):"},{"line_number":47,"context_line":"            server_args \u003d {"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f91af0f_999b1871","line":44,"in_reply_to":"9f91af0f_892194b7","updated":"2018-01-03 08:46:23.000000000","message":"Done","commit_id":"a30091183ccb065010b4deea54074ef15c5d2106"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e30e621ad8fae8a5ed3ae0390439b94241d8cef","unresolved":false,"context_lines":[{"line_number":53,"context_line":"            }"},{"line_number":54,"context_line":"            if ports is not None:"},{"line_number":55,"context_line":"                server_args[\u0027networks\u0027] \u003d [{\u0027uuid\u0027: self.network[\u0027id\u0027],"},{"line_number":56,"context_line":"                                            \u0027port\u0027: ports[i][\u0027id\u0027]}]"},{"line_number":57,"context_line":"            servers.append(self.create_server(**server_args))"},{"line_number":58,"context_line":"        for i, server in enumerate(servers):"},{"line_number":59,"context_line":"            waiters.wait_for_server_status("}],"source_content_type":"text/x-python","patch_set":9,"id":"9f91af0f_497dccaf","line":56,"updated":"2018-01-02 20:10:29.000000000","message":"nit: could be\n\nserver_args[\u0027networks\u0027][0].update({\u0027port\u0027: ports[i][\u0027id\u0027]})","commit_id":"a30091183ccb065010b4deea54074ef15c5d2106"},{"author":{"_account_id":26899,"name":"Roee Agiman","email":"ragiman@redhat.com","username":"ragiman"},"change_message_id":"bc6e1ac0595eb9193e9347318b86faca1c953883","unresolved":false,"context_lines":[{"line_number":53,"context_line":"            }"},{"line_number":54,"context_line":"            if ports is not None:"},{"line_number":55,"context_line":"                server_args[\u0027networks\u0027] \u003d [{\u0027uuid\u0027: self.network[\u0027id\u0027],"},{"line_number":56,"context_line":"                                            \u0027port\u0027: ports[i][\u0027id\u0027]}]"},{"line_number":57,"context_line":"            servers.append(self.create_server(**server_args))"},{"line_number":58,"context_line":"        for i, server in enumerate(servers):"},{"line_number":59,"context_line":"            waiters.wait_for_server_status("}],"source_content_type":"text/x-python","patch_set":9,"id":"9f91af0f_398a0c1b","line":56,"in_reply_to":"9f91af0f_497dccaf","updated":"2018-01-03 08:46:23.000000000","message":"Done","commit_id":"a30091183ccb065010b4deea54074ef15c5d2106"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"3469dcaf34ca820470b2028a9191e663765c05c7","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def create_vm_testing_sec_grp(self, num_servers\u003d2, security_groups\u003dNone,"},{"line_number":44,"context_line":"                                  ports\u003dNone):"},{"line_number":45,"context_line":"        \"\"\"Create instance for security group testing"},{"line_number":46,"context_line":"        "},{"line_number":47,"context_line":"        :param num_servers (int): number of servers to spawn"},{"line_number":48,"context_line":"        :param security_groups (list): list of security groups"},{"line_number":49,"context_line":"        :param ports* (list): list of ports"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f91af0f_cf0f922c","line":46,"range":{"start_line":46,"start_character":0,"end_line":46,"end_character":8},"updated":"2018-01-03 09:52:23.000000000","message":"please remove this spaces :)","commit_id":"b7f183d768d2749a674ccc64e6ed7dd85638a692"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"3469dcaf34ca820470b2028a9191e663765c05c7","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        :param num_servers (int): number of servers to spawn"},{"line_number":48,"context_line":"        :param security_groups (list): list of security groups"},{"line_number":49,"context_line":"        :param ports* (list): list of ports"},{"line_number":50,"context_line":"        "},{"line_number":51,"context_line":"        *Needs to be the same length as num_servers"},{"line_number":52,"context_line":"        \"\"\""},{"line_number":53,"context_line":"        servers, fips, server_ssh_clients \u003d ([], [], [])"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f91af0f_8f198a7a","line":50,"range":{"start_line":50,"start_character":0,"end_line":50,"end_character":8},"updated":"2018-01-03 09:52:23.000000000","message":"ditto","commit_id":"b7f183d768d2749a674ccc64e6ed7dd85638a692"}]}