)]}'
{"patrole_tempest_plugin/tests/api/network/test_security_groups_rbac.py":[{"author":{"_account_id":23625,"name":"Nicolas Helgeson","email":"nh202b@att.com","username":"nhelgeson"},"change_message_id":"0eda5cb5133cbd0dfcd1933a27bd72aa6c5ccf71","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from oslo_log import log"},{"line_number":17,"context_line":"from tempest import config"},{"line_number":18,"context_line":"from tempest.lib.common.utils import data_utils"},{"line_number":19,"context_line":"from tempest.lib.common.utils import test_utils"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_2f2db4a7","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":24},"updated":"2017-02-11 01:06:15.000000000","message":"unused","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23625,"name":"Nicolas Helgeson","email":"nh202b@att.com","username":"nhelgeson"},"change_message_id":"0eda5cb5133cbd0dfcd1933a27bd72aa6c5ccf71","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from patrole_tempest_plugin.tests.api.network import rbac_base as base"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"CONF \u003d config.CONF"},{"line_number":29,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class SecGroupRbacTest(base.BaseNetworkRbacTest):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_4f182885","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":29},"updated":"2017-02-11 01:06:15.000000000","message":"unused","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"d7c893b4c6f9b94901957574a48a2e87671b9b15","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from patrole_tempest_plugin.tests.api.network import rbac_base as base"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"CONF \u003d config.CONF"},{"line_number":29,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class SecGroupRbacTest(base.BaseNetworkRbacTest):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_f1ab481b","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":29},"in_reply_to":"fa31d9ce_4f182885","updated":"2017-02-13 16:42:21.000000000","message":"Will be used once 404 explanations are added.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":37,"context_line":"        cls.securitygroups \u003d []"},{"line_number":38,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":39,"context_line":"        body \u003d cls.security_groups_client.create_security_group("},{"line_number":40,"context_line":"            name\u003dcls.admin_secgroup_name)"},{"line_number":41,"context_line":"        cls.admin_secgroup \u003d body[\u0027security_group\u0027]"},{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_6e189585","line":43,"range":{"start_line":39,"start_character":8,"end_line":43,"end_character":53},"updated":"2017-02-10 21:18:44.000000000","message":"Can we reduce this to:\n\n cls.admin_secgroup \u003d cls.security_groups_client.create_security_group(\n            name\u003dcls.admin_secgroup_name)[\u0027security_group\u0027]\n\nthen instead of cls.admin_secgroup_id, you can just use cls.admin_secgroup[\u0027id\u0027]\n\njust a suggestion - I would just do what is the most consistent with other code that was committed. I have mostly seen what i posted in this comment","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    @classmethod"},{"line_number":35,"context_line":"    def resource_setup(cls):"},{"line_number":36,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":37,"context_line":"        cls.securitygroups \u003d []"},{"line_number":38,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":39,"context_line":"        body \u003d cls.security_groups_client.create_security_group("},{"line_number":40,"context_line":"            name\u003dcls.admin_secgroup_name)"},{"line_number":41,"context_line":"        cls.admin_secgroup \u003d body[\u0027security_group\u0027]"},{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_f5ac2337","line":43,"range":{"start_line":37,"start_character":0,"end_line":43,"end_character":53},"updated":"2017-02-13 12:36:47.000000000","message":"Why can\u0027t we use \"_create_security_group()\" private method here ?","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"81401eba8f78ad593a8b4459ccacd889dbded67b","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":37,"context_line":"        cls.securitygroups \u003d []"},{"line_number":38,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":39,"context_line":"        body \u003d cls.security_groups_client.create_security_group("},{"line_number":40,"context_line":"            name\u003dcls.admin_secgroup_name)"},{"line_number":41,"context_line":"        cls.admin_secgroup \u003d body[\u0027security_group\u0027]"},{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_f24d409a","line":43,"range":{"start_line":39,"start_character":8,"end_line":43,"end_character":53},"in_reply_to":"fa31d9ce_6e189585","updated":"2017-02-11 06:22:23.000000000","message":"That approach is probably better.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"32875ffb1e3416c862b14c7029f50a1b11ca5af0","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    @classmethod"},{"line_number":35,"context_line":"    def resource_setup(cls):"},{"line_number":36,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":37,"context_line":"        cls.securitygroups \u003d []"},{"line_number":38,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":39,"context_line":"        body \u003d cls.security_groups_client.create_security_group("},{"line_number":40,"context_line":"            name\u003dcls.admin_secgroup_name)"},{"line_number":41,"context_line":"        cls.admin_secgroup \u003d body[\u0027security_group\u0027]"},{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_8d96684a","line":43,"range":{"start_line":37,"start_character":0,"end_line":43,"end_character":53},"in_reply_to":"fa31d9ce_717d986d","updated":"2017-02-13 18:56:34.000000000","message":"I am talking about \"create_security_group\" local in this file at line #L58","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    @classmethod"},{"line_number":35,"context_line":"    def resource_setup(cls):"},{"line_number":36,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":37,"context_line":"        cls.securitygroups \u003d []"},{"line_number":38,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":39,"context_line":"        body \u003d cls.security_groups_client.create_security_group("},{"line_number":40,"context_line":"            name\u003dcls.admin_secgroup_name)"},{"line_number":41,"context_line":"        cls.admin_secgroup \u003d body[\u0027security_group\u0027]"},{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_b99179f3","line":43,"range":{"start_line":37,"start_character":0,"end_line":43,"end_character":53},"in_reply_to":"fa31d9ce_8d96684a","updated":"2017-02-14 20:34:05.000000000","message":"Mohd: _create_security_group at #L58 uses self.addCleanup which cannot be used inside a classmethod.\n\nDavid: remove addCleanup in _create_security_group and instead use self.securitygroups.append(group_create_body[\u0027security_group\u0027]) and then convert _create_security_group into a classmethod and call it from this function (after declaring cls.securitygroups of course).","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"77d204a6c92c72bd1ec30280d9e048fd6b7ebc85","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    @classmethod"},{"line_number":35,"context_line":"    def resource_setup(cls):"},{"line_number":36,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":37,"context_line":"        cls.securitygroups \u003d []"},{"line_number":38,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":39,"context_line":"        body \u003d cls.security_groups_client.create_security_group("},{"line_number":40,"context_line":"            name\u003dcls.admin_secgroup_name)"},{"line_number":41,"context_line":"        cls.admin_secgroup \u003d body[\u0027security_group\u0027]"},{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_b9fa5941","line":43,"range":{"start_line":37,"start_character":0,"end_line":43,"end_character":53},"in_reply_to":"fa31d9ce_8d96684a","updated":"2017-02-14 20:29:31.000000000","message":"That method is for test case level resources.  The addCleanup is at the end of each test case.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"d7c893b4c6f9b94901957574a48a2e87671b9b15","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    @classmethod"},{"line_number":35,"context_line":"    def resource_setup(cls):"},{"line_number":36,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":37,"context_line":"        cls.securitygroups \u003d []"},{"line_number":38,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":39,"context_line":"        body \u003d cls.security_groups_client.create_security_group("},{"line_number":40,"context_line":"            name\u003dcls.admin_secgroup_name)"},{"line_number":41,"context_line":"        cls.admin_secgroup \u003d body[\u0027security_group\u0027]"},{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_717d986d","line":43,"range":{"start_line":37,"start_character":0,"end_line":43,"end_character":53},"in_reply_to":"fa31d9ce_f5ac2337","updated":"2017-02-13 16:42:21.000000000","message":"_create_security_group() is part of the BaseSecGroupTest, which this test does not extend.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"},{"line_number":47,"context_line":"        # Clean up security groups"},{"line_number":48,"context_line":"        for securitygroup in cls.securitygroups:"},{"line_number":49,"context_line":"            test_utils.call_and_ignore_notfound_exc("},{"line_number":50,"context_line":"                cls.security_groups_client.delete_security_group,"},{"line_number":51,"context_line":"                securitygroup[\u0027id\u0027])"},{"line_number":52,"context_line":"        super(SecGroupRbacTest, cls).resource_cleanup()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def tearDown(self):"},{"line_number":55,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_357ebbae","line":52,"range":{"start_line":45,"start_character":0,"end_line":52,"end_character":55},"updated":"2017-02-13 12:36:47.000000000","message":"If implemented as of above comment, this method is not required.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"d7c893b4c6f9b94901957574a48a2e87671b9b15","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"},{"line_number":47,"context_line":"        # Clean up security groups"},{"line_number":48,"context_line":"        for securitygroup in cls.securitygroups:"},{"line_number":49,"context_line":"            test_utils.call_and_ignore_notfound_exc("},{"line_number":50,"context_line":"                cls.security_groups_client.delete_security_group,"},{"line_number":51,"context_line":"                securitygroup[\u0027id\u0027])"},{"line_number":52,"context_line":"        super(SecGroupRbacTest, cls).resource_cleanup()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def tearDown(self):"},{"line_number":55,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_f191889a","line":52,"range":{"start_line":45,"start_character":0,"end_line":52,"end_character":55},"in_reply_to":"fa31d9ce_357ebbae","updated":"2017-02-13 16:42:21.000000000","message":"see above reply","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"32875ffb1e3416c862b14c7029f50a1b11ca5af0","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        cls.admin_secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":43,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def resource_cleanup(cls):"},{"line_number":47,"context_line":"        # Clean up security groups"},{"line_number":48,"context_line":"        for securitygroup in cls.securitygroups:"},{"line_number":49,"context_line":"            test_utils.call_and_ignore_notfound_exc("},{"line_number":50,"context_line":"                cls.security_groups_client.delete_security_group,"},{"line_number":51,"context_line":"                securitygroup[\u0027id\u0027])"},{"line_number":52,"context_line":"        super(SecGroupRbacTest, cls).resource_cleanup()"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def tearDown(self):"},{"line_number":55,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_ad916c54","line":52,"range":{"start_line":45,"start_character":0,"end_line":52,"end_character":55},"in_reply_to":"fa31d9ce_f191889a","updated":"2017-02-13 18:56:34.000000000","message":"see reply above","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    def _create_security_group(self):"},{"line_number":59,"context_line":"        # Create a security group"},{"line_number":60,"context_line":"        name \u003d data_utils.rand_name(\u0027secgroup-\u0027)"},{"line_number":61,"context_line":"        group_create_body \u003d ("},{"line_number":62,"context_line":"            self.security_groups_client.create_security_group(name\u003dname))"},{"line_number":63,"context_line":"        self.addCleanup("},{"line_number":64,"context_line":"            test_utils.call_and_ignore_notfound_exc,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_1508d72b","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":25},"updated":"2017-02-13 12:36:47.000000000","message":"just write \"security_group \u003d\"","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    def _create_security_group(self):"},{"line_number":59,"context_line":"        # Create a security group"},{"line_number":60,"context_line":"        name \u003d data_utils.rand_name(\u0027secgroup-\u0027)"},{"line_number":61,"context_line":"        group_create_body \u003d ("},{"line_number":62,"context_line":"            self.security_groups_client.create_security_group(name\u003dname))"},{"line_number":63,"context_line":"        self.addCleanup("},{"line_number":64,"context_line":"            test_utils.call_and_ignore_notfound_exc,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_7979c1c7","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":25},"in_reply_to":"fa31d9ce_1508d72b","updated":"2017-02-14 20:34:05.000000000","message":"This wouldn\u0027t work. Needs to be \"group_create_body \u003d \\\" on the first line if you mean \"get rid of the parentheses\".\n\nIf you\u0027re saying just change the name of the variable, I disagree; that\u0027s trivial.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            test_utils.call_and_ignore_notfound_exc,"},{"line_number":65,"context_line":"            self.security_groups_client.delete_security_group,"},{"line_number":66,"context_line":"            group_create_body[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":67,"context_line":"        return group_create_body, name"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":70,"context_line":"                                 rule\u003d\"create_security_group\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_184d6e6c","line":67,"range":{"start_line":67,"start_character":15,"end_line":67,"end_character":38},"updated":"2017-02-13 12:36:47.000000000","message":"Tuple like return is not needed. Wherever one wants to use security group name, he/she can directly call it as security_group[\u0027name\u0027] or like.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            test_utils.call_and_ignore_notfound_exc,"},{"line_number":65,"context_line":"            self.security_groups_client.delete_security_group,"},{"line_number":66,"context_line":"            group_create_body[\u0027security_group\u0027][\u0027id\u0027])"},{"line_number":67,"context_line":"        return group_create_body, name"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":70,"context_line":"                                 rule\u003d\"create_security_group\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_3942e968","line":67,"range":{"start_line":67,"start_character":15,"end_line":67,"end_character":38},"in_reply_to":"fa31d9ce_184d6e6c","updated":"2017-02-14 20:34:05.000000000","message":"I agree with this, it\u0027s neater.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":84,"context_line":"            body \u003d self.security_groups_client.show_security_group("},{"line_number":85,"context_line":"                self.admin_secgroup_id)"},{"line_number":86,"context_line":"            security_group \u003d body[\u0027security_group\u0027]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"            self.assertEqual(self.admin_secgroup_id, security_group[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_4932ebf2","line":85,"range":{"start_line":85,"start_character":16,"end_line":85,"end_character":38},"updated":"2017-02-10 21:18:44.000000000","message":"or could be\n\nself.admin_secgroup[\u0027id\u0027]\n\nagain - up to you","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":84,"context_line":"            body \u003d self.security_groups_client.show_security_group("},{"line_number":85,"context_line":"                self.admin_secgroup_id)"},{"line_number":86,"context_line":"            security_group \u003d body[\u0027security_group\u0027]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"            self.assertEqual(self.admin_secgroup_id, security_group[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_7960e101","line":85,"range":{"start_line":85,"start_character":16,"end_line":85,"end_character":38},"in_reply_to":"fa31d9ce_4932ebf2","updated":"2017-02-14 20:34:05.000000000","message":"I\u0027ve seen it done either way. I think it\u0027s fine as is.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                self.admin_secgroup_id)"},{"line_number":86,"context_line":"            security_group \u003d body[\u0027security_group\u0027]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"            self.assertEqual(self.admin_secgroup_id, security_group[\u0027id\u0027])"},{"line_number":89,"context_line":"            self.assertEqual(self.admin_secgroup_name, security_group[\u0027name\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        except exceptions.NotFound as e:"},{"line_number":92,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_ce88a1ea","line":89,"range":{"start_line":88,"start_character":12,"end_line":89,"end_character":78},"updated":"2017-02-10 21:18:44.000000000","message":"are asserts needed in RBAC testing? Internally we decided not to use them","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"81401eba8f78ad593a8b4459ccacd889dbded67b","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                self.admin_secgroup_id)"},{"line_number":86,"context_line":"            security_group \u003d body[\u0027security_group\u0027]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"            self.assertEqual(self.admin_secgroup_id, security_group[\u0027id\u0027])"},{"line_number":89,"context_line":"            self.assertEqual(self.admin_secgroup_name, security_group[\u0027name\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        except exceptions.NotFound as e:"},{"line_number":92,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_d2311c0d","line":89,"range":{"start_line":88,"start_character":12,"end_line":89,"end_character":78},"in_reply_to":"fa31d9ce_ce88a1ea","updated":"2017-02-11 06:22:23.000000000","message":"They are not, I\u0027ll remove.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                self.admin_secgroup_id)"},{"line_number":86,"context_line":"            security_group \u003d body[\u0027security_group\u0027]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"            self.assertEqual(self.admin_secgroup_id, security_group[\u0027id\u0027])"},{"line_number":89,"context_line":"            self.assertEqual(self.admin_secgroup_name, security_group[\u0027name\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        except exceptions.NotFound as e:"},{"line_number":92,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_b956d927","line":89,"range":{"start_line":88,"start_character":12,"end_line":89,"end_character":78},"in_reply_to":"fa31d9ce_d2311c0d","updated":"2017-02-14 20:34:05.000000000","message":"David: Agreed.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            self.assertEqual(self.admin_secgroup_id, security_group[\u0027id\u0027])"},{"line_number":89,"context_line":"            self.assertEqual(self.admin_secgroup_name, security_group[\u0027name\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        except exceptions.NotFound as e:"},{"line_number":92,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_d57d5f1d","line":91,"range":{"start_line":91,"start_character":26,"end_line":91,"end_character":35},"updated":"2017-02-13 12:36:47.000000000","message":"Required information on this should be logged.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"d7c893b4c6f9b94901957574a48a2e87671b9b15","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            self.assertEqual(self.admin_secgroup_id, security_group[\u0027id\u0027])"},{"line_number":89,"context_line":"            self.assertEqual(self.admin_secgroup_name, security_group[\u0027name\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        except exceptions.NotFound as e:"},{"line_number":92,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_b1c5508f","line":91,"range":{"start_line":91,"start_character":26,"end_line":91,"end_character":35},"in_reply_to":"fa31d9ce_d57d5f1d","updated":"2017-02-13 16:42:21.000000000","message":"Agreed, thanks for noticing.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"c4175bb41243a4f8f79a6d5e25078ab865164345","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        # Create a security group"},{"line_number":100,"context_line":"        name \u003d data_utils.rand_name(\u0027secgroup\u0027)"},{"line_number":101,"context_line":"        body \u003d self.security_groups_client.create_security_group(name\u003dname)"},{"line_number":102,"context_line":"        secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_e968ffbc","line":101,"range":{"start_line":101,"start_character":15,"end_line":101,"end_character":75},"updated":"2017-02-10 21:19:10.000000000","message":"Why not use self._create_security_group() and then remove the self.addCleanup below?","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        # Create a security group"},{"line_number":100,"context_line":"        name \u003d data_utils.rand_name(\u0027secgroup\u0027)"},{"line_number":101,"context_line":"        body \u003d self.security_groups_client.create_security_group(name\u003dname)"},{"line_number":102,"context_line":"        secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_8ecce936","line":101,"range":{"start_line":101,"start_character":7,"end_line":101,"end_character":75},"updated":"2017-02-10 21:18:44.000000000","message":"should the addCleanup be placed right after the create?","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"81401eba8f78ad593a8b4459ccacd889dbded67b","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        # Create a security group"},{"line_number":100,"context_line":"        name \u003d data_utils.rand_name(\u0027secgroup\u0027)"},{"line_number":101,"context_line":"        body \u003d self.security_groups_client.create_security_group(name\u003dname)"},{"line_number":102,"context_line":"        secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_722150b4","line":101,"range":{"start_line":101,"start_character":15,"end_line":101,"end_character":75},"in_reply_to":"fa31d9ce_e968ffbc","updated":"2017-02-11 06:22:23.000000000","message":"Both good comments, will go with Felipe\u0027s suggestion.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":105,"context_line":"        try:"},{"line_number":106,"context_line":"            self.security_groups_client.delete_security_group(secgroup_id)"},{"line_number":107,"context_line":"            self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":108,"context_line":"                            self.security_groups_client.delete_security_group,"},{"line_number":109,"context_line":"                            secgroup_id)"},{"line_number":110,"context_line":"        except exceptions.NotFound as e:"},{"line_number":111,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_694def71","line":109,"range":{"start_line":107,"start_character":12,"end_line":109,"end_character":40},"updated":"2017-02-10 21:18:44.000000000","message":"see note right above","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":108,"context_line":"                            self.security_groups_client.delete_security_group,"},{"line_number":109,"context_line":"                            secgroup_id)"},{"line_number":110,"context_line":"        except exceptions.NotFound as e:"},{"line_number":111,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_75baf3c1","line":110,"range":{"start_line":110,"start_character":26,"end_line":110,"end_character":34},"updated":"2017-02-13 12:36:47.000000000","message":"Ditto","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":116,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":119,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":120,"context_line":"            security_group_id\u003dself.admin_secgroup_id,"},{"line_number":121,"context_line":"            direction\u003d\u0027ingress\u0027,"},{"line_number":122,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":123,"context_line":"            port_range_min\u003d99,"},{"line_number":124,"context_line":"            port_range_max\u003d99)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        sec_group_rule \u003d body[\u0027security_group_rule\u0027]"},{"line_number":127,"context_line":"        self.addCleanup("}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_3546bbbf","line":124,"range":{"start_line":119,"start_character":8,"end_line":124,"end_character":30},"updated":"2017-02-13 12:36:47.000000000","message":"As these lines are being used multiple times, it is better to create a private method to create sec group rules with default parameters.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":116,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":119,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":120,"context_line":"            security_group_id\u003dself.admin_secgroup_id,"},{"line_number":121,"context_line":"            direction\u003d\u0027ingress\u0027,"},{"line_number":122,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":123,"context_line":"            port_range_min\u003d99,"},{"line_number":124,"context_line":"            port_range_max\u003d99)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        sec_group_rule \u003d body[\u0027security_group_rule\u0027]"},{"line_number":127,"context_line":"        self.addCleanup("}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_6ebc156d","line":124,"range":{"start_line":119,"start_character":8,"end_line":124,"end_character":30},"updated":"2017-02-10 21:18:44.000000000","message":"or could be:\n\nsec_group_rule \u003d self.security_group_rules_client.create_security_group_rule(\n            security_group_id\u003dself.admin_secgroup_id,\n            direction\u003d\u0027ingress\u0027,\n            protocol\u003d\u0027tcp\u0027,\n            port_range_min\u003d99,\n            port_range_max\u003d99)[\u0027security_group_rule\u0027]","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":116,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":119,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":120,"context_line":"            security_group_id\u003dself.admin_secgroup_id,"},{"line_number":121,"context_line":"            direction\u003d\u0027ingress\u0027,"},{"line_number":122,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":123,"context_line":"            port_range_min\u003d99,"},{"line_number":124,"context_line":"            port_range_max\u003d99)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        sec_group_rule \u003d body[\u0027security_group_rule\u0027]"},{"line_number":127,"context_line":"        self.addCleanup("}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_39102959","line":124,"range":{"start_line":119,"start_character":8,"end_line":124,"end_character":30},"in_reply_to":"fa31d9ce_3546bbbf","updated":"2017-02-14 20:34:05.000000000","message":"Yes, that\u0027s a good idea. Especially because of the addCleanup as you pointed out below.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23184,"name":"DavidPurcell","email":"d.purcell222@gmail.com","username":"DavidPurcell"},"change_message_id":"81401eba8f78ad593a8b4459ccacd889dbded67b","unresolved":false,"context_lines":[{"line_number":116,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":119,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":120,"context_line":"            security_group_id\u003dself.admin_secgroup_id,"},{"line_number":121,"context_line":"            direction\u003d\u0027ingress\u0027,"},{"line_number":122,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":123,"context_line":"            port_range_min\u003d99,"},{"line_number":124,"context_line":"            port_range_max\u003d99)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        sec_group_rule \u003d body[\u0027security_group_rule\u0027]"},{"line_number":127,"context_line":"        self.addCleanup("}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_f21b807e","line":124,"range":{"start_line":119,"start_character":8,"end_line":124,"end_character":30},"in_reply_to":"fa31d9ce_6ebc156d","updated":"2017-02-11 06:22:23.000000000","message":"Done","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            port_range_max\u003d99)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        sec_group_rule \u003d body[\u0027security_group_rule\u0027]"},{"line_number":127,"context_line":"        self.addCleanup("},{"line_number":128,"context_line":"            test_utils.call_and_ignore_notfound_exc,"},{"line_number":129,"context_line":"            self.security_group_rules_client.delete_security_group_rule,"},{"line_number":130,"context_line":"            sec_group_rule[\u0027id\u0027])"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":133,"context_line":"                                 rule\u003d\"delete_security_group_rule\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_757d93e6","line":130,"range":{"start_line":127,"start_character":0,"end_line":130,"end_character":33},"updated":"2017-02-13 12:36:47.000000000","message":"Private secgroup rule method can eliminate these lines too.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":141,"context_line":"            port_range_min\u003d77,"},{"line_number":142,"context_line":"            port_range_max\u003d77)"},{"line_number":143,"context_line":"        sec_group_rule_id \u003d body[\u0027security_group_rule\u0027][\u0027id\u0027]"},{"line_number":144,"context_line":"        self.addCleanup("},{"line_number":145,"context_line":"            test_utils.call_and_ignore_notfound_exc,"},{"line_number":146,"context_line":"            self.security_group_rules_client.delete_security_group_rule,"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_a96e07c7","line":143,"range":{"start_line":143,"start_character":8,"end_line":143,"end_character":61},"updated":"2017-02-10 21:18:44.000000000","message":"see notes above","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":134,"context_line":"    @decorators.idempotent_id(\u00272262539e-b7d9-438c-acf9-a5ce0613be28\u0027)"},{"line_number":135,"context_line":"    def test_delete_security_group_rule(self):"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":138,"context_line":"            security_group_id\u003dself.admin_secgroup_id,"},{"line_number":139,"context_line":"            direction\u003d\u0027ingress\u0027,"},{"line_number":140,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":141,"context_line":"            port_range_min\u003d77,"},{"line_number":142,"context_line":"            port_range_max\u003d77)"},{"line_number":143,"context_line":"        sec_group_rule_id \u003d body[\u0027security_group_rule\u0027][\u0027id\u0027]"},{"line_number":144,"context_line":"        self.addCleanup("},{"line_number":145,"context_line":"            test_utils.call_and_ignore_notfound_exc,"},{"line_number":146,"context_line":"            self.security_group_rules_client.delete_security_group_rule,"},{"line_number":147,"context_line":"            sec_group_rule_id)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_f5698326","line":147,"range":{"start_line":137,"start_character":0,"end_line":147,"end_character":30},"updated":"2017-02-13 12:36:47.000000000","message":"Same as above.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":134,"context_line":"    @decorators.idempotent_id(\u00272262539e-b7d9-438c-acf9-a5ce0613be28\u0027)"},{"line_number":135,"context_line":"    def test_delete_security_group_rule(self):"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":138,"context_line":"            security_group_id\u003dself.admin_secgroup_id,"},{"line_number":139,"context_line":"            direction\u003d\u0027ingress\u0027,"},{"line_number":140,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":141,"context_line":"            port_range_min\u003d77,"},{"line_number":142,"context_line":"            port_range_max\u003d77)"},{"line_number":143,"context_line":"        sec_group_rule_id \u003d body[\u0027security_group_rule\u0027][\u0027id\u0027]"},{"line_number":144,"context_line":"        self.addCleanup("},{"line_number":145,"context_line":"            test_utils.call_and_ignore_notfound_exc,"},{"line_number":146,"context_line":"            self.security_group_rules_client.delete_security_group_rule,"},{"line_number":147,"context_line":"            sec_group_rule_id)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_990bd529","line":147,"range":{"start_line":137,"start_character":0,"end_line":147,"end_character":30},"in_reply_to":"fa31d9ce_f5698326","updated":"2017-02-14 20:34:05.000000000","message":"Agreed.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"3dbefe05cfa509c54e2610c6816b4e7df43368a0","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            sec_group_rule_id)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        try:"},{"line_number":152,"context_line":"            self.security_group_rules_client.delete_security_group_rule("},{"line_number":153,"context_line":"                sec_group_rule_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_c9aa5bd4","line":150,"updated":"2017-02-10 21:18:44.000000000","message":"delete space to stay consistent with other code in the file","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a8d76f1c6bd3592c120551ed69d709be31662fb6","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            sec_group_rule_id)"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        try:"},{"line_number":152,"context_line":"            self.security_group_rules_client.delete_security_group_rule("},{"line_number":153,"context_line":"                sec_group_rule_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_5915dd46","line":150,"in_reply_to":"fa31d9ce_c9aa5bd4","updated":"2017-02-14 20:34:05.000000000","message":"Agreed.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a72c6614644ad7bfb53020f59ac6c4c09d17dc8f","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        try:"},{"line_number":152,"context_line":"            self.security_group_rules_client.delete_security_group_rule("},{"line_number":153,"context_line":"                sec_group_rule_id)"},{"line_number":154,"context_line":"        except exceptions.NotFound as e:"},{"line_number":155,"context_line":"            raise rbac_exceptions.RbacActionFailed(e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_d5555f50","line":154,"range":{"start_line":154,"start_character":26,"end_line":154,"end_character":34},"updated":"2017-02-13 12:36:47.000000000","message":"Proper message of 404 irregularity should be logged for proper analysis.","commit_id":"2d27b9d67278589e87745f7eb139c1998ae35c61"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"b538deea137c4116b81f81988146df46918324e1","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    @decorators.idempotent_id(\u00270b1330fd-dd28-40f3-ad73-966052e4b3de\u0027)"},{"line_number":94,"context_line":"    def test_delete_security_group(self):"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        # Create a security group"},{"line_number":97,"context_line":"        name \u003d data_utils.rand_name(\u0027secgroup\u0027)"},{"line_number":98,"context_line":"        body \u003d self.security_groups_client.create_security_group(name\u003dname)"},{"line_number":99,"context_line":"        secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":100,"context_line":"        self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":101,"context_line":"                        self.security_groups_client.delete_security_group,"},{"line_number":102,"context_line":"                        secgroup_id)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":105,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_06e75940","line":102,"range":{"start_line":96,"start_character":0,"end_line":102,"end_character":36},"updated":"2017-02-15 14:57:32.000000000","message":"Just one thing, \nWhy not we should reuse method \"_create_security_group\" for these lines ?","commit_id":"7d041ba9591946a5b08cf313911aaa680014d124"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"620b95a25ee689028242c3b988dfdbd5afd4f5bc","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    @decorators.idempotent_id(\u00270b1330fd-dd28-40f3-ad73-966052e4b3de\u0027)"},{"line_number":94,"context_line":"    def test_delete_security_group(self):"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        # Create a security group"},{"line_number":97,"context_line":"        name \u003d data_utils.rand_name(\u0027secgroup\u0027)"},{"line_number":98,"context_line":"        body \u003d self.security_groups_client.create_security_group(name\u003dname)"},{"line_number":99,"context_line":"        secgroup_id \u003d body[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":100,"context_line":"        self.addCleanup(test_utils.call_and_ignore_notfound_exc,"},{"line_number":101,"context_line":"                        self.security_groups_client.delete_security_group,"},{"line_number":102,"context_line":"                        secgroup_id)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":105,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_1c440cb5","line":102,"range":{"start_line":96,"start_character":0,"end_line":102,"end_character":36},"in_reply_to":"fa31d9ce_06e75940","updated":"2017-02-15 15:40:00.000000000","message":"Agreed.","commit_id":"7d041ba9591946a5b08cf313911aaa680014d124"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"b538deea137c4116b81f81988146df46918324e1","unresolved":false,"context_lines":[{"line_number":116,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":119,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":120,"context_line":"            security_group_id\u003dself.admin_secgroup[\u0027id\u0027],"},{"line_number":121,"context_line":"            direction\u003d\u0027ingress\u0027,"},{"line_number":122,"context_line":"            protocol\u003d\u0027tcp\u0027,"},{"line_number":123,"context_line":"            port_range_min\u003d99,"},{"line_number":124,"context_line":"            port_range_max\u003d99)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        sec_group_rule \u003d body[\u0027security_group_rule\u0027]"},{"line_number":127,"context_line":"        self.addCleanup("},{"line_number":128,"context_line":"            test_utils.call_and_ignore_notfound_exc,"},{"line_number":129,"context_line":"            self.security_group_rules_client.delete_security_group_rule,"},{"line_number":130,"context_line":"            sec_group_rule[\u0027id\u0027])"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":133,"context_line":"                                 rule\u003d\"delete_security_group_rule\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_e6e5e508","line":130,"range":{"start_line":119,"start_character":0,"end_line":130,"end_character":33},"updated":"2017-02-15 14:57:32.000000000","message":"As these lines are being used in few more test cases, a private method say \"_create_security_group_rule(self, security_group, **kwargs)\" can also be created for these lines. and thus we can reduce significant LOCs.\n\nAlso if added more test cases which require rule creation, this created method can be used.","commit_id":"7d041ba9591946a5b08cf313911aaa680014d124"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"c96d82f00fa3970a780feac7b3f4bf3a3abd0f5d","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    @decorators.idempotent_id(\u0027953d78df-00cd-416f-9cbd-b7cb4ea65772\u0027)"},{"line_number":111,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":114,"context_line":"        body \u003d self.security_group_rules_client.create_security_group_rule("},{"line_number":115,"context_line":"            security_group_id\u003dself.admin_secgroup[\u0027id\u0027],"},{"line_number":116,"context_line":"            direction\u003d\u0027ingress\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa31d9ce_0d9acec5","line":113,"updated":"2017-02-17 00:51:58.000000000","message":"a bit inconsistent with LN132\nas Mh Raies said on the previous patch this probably could be factored out since used on 2 test cases.","commit_id":"37c67d532efe64ca85fef30d4a55574b790983e2"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"f11fb929e0cc2ec2e61d1e9a2e85e4f92ca3579f","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from oslo_log import log"},{"line_number":17,"context_line":"from tempest import config"},{"line_number":18,"context_line":"from tempest.lib.common.utils import data_utils"},{"line_number":19,"context_line":"from tempest.lib.common.utils import test_utils"},{"line_number":20,"context_line":"from tempest.lib import decorators"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_d373f2af","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":26},"updated":"2017-02-20 19:20:40.000000000","message":"Unused","commit_id":"902ca0bee85fecc58ceaccff97b7995288db7d6f"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"f11fb929e0cc2ec2e61d1e9a2e85e4f92ca3579f","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from patrole_tempest_plugin.rbac_utils import rbac_utils"},{"line_number":26,"context_line":"from patrole_tempest_plugin.tests.api.network import rbac_base as base"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"CONF \u003d config.CONF"},{"line_number":29,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_b36e7ec7","line":28,"range":{"start_line":28,"start_character":0,"end_line":28,"end_character":18},"updated":"2017-02-20 19:20:40.000000000","message":"Unused","commit_id":"902ca0bee85fecc58ceaccff97b7995288db7d6f"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"f11fb929e0cc2ec2e61d1e9a2e85e4f92ca3579f","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    def resource_setup(cls):"},{"line_number":40,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":41,"context_line":"        cls.securitygroups \u003d []"},{"line_number":42,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":43,"context_line":"        cls.admin_secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":44,"context_line":"            name\u003dcls.admin_secgroup_name)[\u0027security_group\u0027]"},{"line_number":45,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_f34b9629","line":42,"range":{"start_line":42,"start_character":8,"end_line":42,"end_character":31},"updated":"2017-02-20 19:20:40.000000000","message":"Strictly speaking, this doesn\u0027t need to a be class-wide attribute, but obviously there\u0027s no real problem keeping it like this.","commit_id":"902ca0bee85fecc58ceaccff97b7995288db7d6f"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"f11fb929e0cc2ec2e61d1e9a2e85e4f92ca3579f","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":41,"context_line":"        cls.securitygroups \u003d []"},{"line_number":42,"context_line":"        cls.admin_secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":43,"context_line":"        cls.admin_secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":44,"context_line":"            name\u003dcls.admin_secgroup_name)[\u0027security_group\u0027]"},{"line_number":45,"context_line":"        cls.securitygroups.append(cls.admin_secgroup)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_13c2ea85","line":44,"range":{"start_line":43,"start_character":8,"end_line":44,"end_character":59},"updated":"2017-02-20 19:20:40.000000000","message":"I\u0027m curious why these are called admin_secgroup? It doesn\u0027t seem like these are any different than any other security group you\u0027ve defined across these tests.","commit_id":"902ca0bee85fecc58ceaccff97b7995288db7d6f"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"5fa3e07225b4222fc9be5d38de8f534f22c09d09","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":132,"context_line":"        sec_group_rule \u003d\\"},{"line_number":133,"context_line":"            self.security_group_rules_client.create_security_group_rule("},{"line_number":134,"context_line":"                security_group_id\u003dself.secgroup[\u0027id\u0027],"},{"line_number":135,"context_line":"                direction\u003d\u0027ingress\u0027,"},{"line_number":136,"context_line":"                protocol\u003d\u0027tcp\u0027,"},{"line_number":137,"context_line":"                port_range_min\u003d99,"},{"line_number":138,"context_line":"                port_range_max\u003d99)[\u0027security_group_rule\u0027]"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"        self.addCleanup("},{"line_number":141,"context_line":"            test_utils.call_and_ignore_notfound_exc,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da36d5c6_feb204e4","line":138,"range":{"start_line":132,"start_character":8,"end_line":138,"end_character":57},"updated":"2017-02-23 16:35:40.000000000","message":"As Mh Raies mentions in previous review it may be better to create a private _create_security_group_rule method?","commit_id":"89130648bbc2f298b3fee4b61658a61afeb6f336"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"46b689685d99c06476676063c015ba9b8008a84c","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":132,"context_line":"        sec_group_rule \u003d\\"},{"line_number":133,"context_line":"            self.security_group_rules_client.create_security_group_rule("},{"line_number":134,"context_line":"                security_group_id\u003dself.secgroup[\u0027id\u0027],"},{"line_number":135,"context_line":"                direction\u003d\u0027ingress\u0027,"},{"line_number":136,"context_line":"                protocol\u003d\u0027tcp\u0027,"},{"line_number":137,"context_line":"                port_range_min\u003d99,"},{"line_number":138,"context_line":"                port_range_max\u003d99)[\u0027security_group_rule\u0027]"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"        self.addCleanup("},{"line_number":141,"context_line":"            test_utils.call_and_ignore_notfound_exc,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da36d5c6_1e60b800","line":138,"range":{"start_line":132,"start_character":8,"end_line":138,"end_character":57},"in_reply_to":"da36d5c6_bec24ce7","updated":"2017-02-23 16:42:59.000000000","message":"I agree with previous review comments.There is no need for repeated code. This should be changed.","commit_id":"89130648bbc2f298b3fee4b61658a61afeb6f336"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"c2b0cd13b426c1412231882634d569c2807fc8cd","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def test_create_security_group_rule(self):"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dTrue)"},{"line_number":132,"context_line":"        sec_group_rule \u003d\\"},{"line_number":133,"context_line":"            self.security_group_rules_client.create_security_group_rule("},{"line_number":134,"context_line":"                security_group_id\u003dself.secgroup[\u0027id\u0027],"},{"line_number":135,"context_line":"                direction\u003d\u0027ingress\u0027,"},{"line_number":136,"context_line":"                protocol\u003d\u0027tcp\u0027,"},{"line_number":137,"context_line":"                port_range_min\u003d99,"},{"line_number":138,"context_line":"                port_range_max\u003d99)[\u0027security_group_rule\u0027]"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"        self.addCleanup("},{"line_number":141,"context_line":"            test_utils.call_and_ignore_notfound_exc,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da36d5c6_bec24ce7","line":138,"range":{"start_line":132,"start_character":8,"end_line":138,"end_character":57},"in_reply_to":"da36d5c6_feb204e4","updated":"2017-02-23 16:40:55.000000000","message":"I mentioned this as well, but it was basically decided that \"it\u0027s not absolutely necessary.\" Also, I think it\u0027s not ideal design, especially given that the port ranges change from 99 to 77 below, for no apparent reason. It just makes it harder to comprehend the code -- if a nuance is introduced, it should have a purpose.","commit_id":"89130648bbc2f298b3fee4b61658a61afeb6f336"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"f4074f7d92e202f4ad7f1ab91207ef66690dac8f","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dFalse)"},{"line_number":35,"context_line":"        super(SecGroupRbacTest, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @classmethod"},{"line_number":38,"context_line":"    def resource_setup(cls):"},{"line_number":39,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":40,"context_line":"        cls.securitygroups \u003d []"},{"line_number":41,"context_line":"        secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":42,"context_line":"        cls.secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":43,"context_line":"            name\u003dsecgroup_name)[\u0027security_group\u0027]"},{"line_number":44,"context_line":"        cls.securitygroups.append(cls.secgroup)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    @classmethod"},{"line_number":47,"context_line":"    def resource_cleanup(cls):"},{"line_number":48,"context_line":"        # Clean up security groups"},{"line_number":49,"context_line":"        for securitygroup in cls.securitygroups:"},{"line_number":50,"context_line":"            test_utils.call_and_ignore_notfound_exc("},{"line_number":51,"context_line":"                cls.security_groups_client.delete_security_group,"},{"line_number":52,"context_line":"                securitygroup[\u0027id\u0027])"},{"line_number":53,"context_line":"        super(SecGroupRbacTest, cls).resource_cleanup()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _create_security_group(self):"},{"line_number":56,"context_line":"        # Create a security group"}],"source_content_type":"text/x-python","patch_set":8,"id":"da36d5c6_2d7e2c19","line":53,"range":{"start_line":37,"start_character":0,"end_line":53,"end_character":55},"updated":"2017-02-23 18:45:16.000000000","message":"This all can be moved in a constructor and can be implemented in 2-3 lines -\n\ndef __init__(self):\n    self.secgroup \u003d _create_security_group()","commit_id":"cd2888db550b0a621027baf9ed9ccad98d087fe1"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"db7c7364db5d8dd8b8f92b1d5bde0dfb67c581cd","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    @classmethod"},{"line_number":38,"context_line":"    def resource_setup(cls):"},{"line_number":39,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":40,"context_line":"        secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":41,"context_line":"        cls.secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":42,"context_line":"            name\u003dsecgroup_name)[\u0027security_group\u0027]"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da36d5c6_5143ed41","line":40,"range":{"start_line":40,"start_character":45,"end_line":40,"end_character":65},"updated":"2017-02-24 04:01:30.000000000","message":"nit: \u0027rbac-admin-secgroup\u0027 is not the best name for this; \u0027admin\u0027 is misleading as these aren\u0027t admin tests.","commit_id":"27e09d97d6a032993f79ca1800e16e1710eba195"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"db7c7364db5d8dd8b8f92b1d5bde0dfb67c581cd","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def resource_setup(cls):"},{"line_number":39,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":40,"context_line":"        secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":41,"context_line":"        cls.secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":42,"context_line":"            name\u003dsecgroup_name)[\u0027security_group\u0027]"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":9,"id":"da36d5c6_317119a9","line":42,"range":{"start_line":41,"start_character":8,"end_line":42,"end_character":49},"updated":"2017-02-24 04:01:30.000000000","message":"L40 - L42 can be replaced with _create_security_group if it\u0027s changed to a classmethod.","commit_id":"27e09d97d6a032993f79ca1800e16e1710eba195"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"a1f0b26110a0431519565ec6aba59f510bfb2b27","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dFalse)"},{"line_number":35,"context_line":"        super(SecGroupRbacTest, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @classmethod"},{"line_number":38,"context_line":"    def resource_setup(cls):"},{"line_number":39,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":40,"context_line":"        secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":41,"context_line":"        cls.secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":42,"context_line":"            name\u003dsecgroup_name)[\u0027security_group\u0027]"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def resource_cleanup(cls):"},{"line_number":46,"context_line":"        # Clean up security group"},{"line_number":47,"context_line":"        test_utils.call_and_ignore_notfound_exc("},{"line_number":48,"context_line":"            cls.security_groups_client.delete_security_group,"},{"line_number":49,"context_line":"            cls.secgroup[\u0027id\u0027])"},{"line_number":50,"context_line":"        super(SecGroupRbacTest, cls).resource_cleanup()"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _create_security_group(self):"},{"line_number":53,"context_line":"        # Create a security group"}],"source_content_type":"text/x-python","patch_set":9,"id":"da36d5c6_274b965a","line":50,"range":{"start_line":37,"start_character":0,"end_line":50,"end_character":55},"updated":"2017-02-24 07:30:20.000000000","message":"As in my previous comment, this entire selected code can be replaced by -\ndef __init__(self):\n    self.secgroup \u003d self._create_security_group()","commit_id":"27e09d97d6a032993f79ca1800e16e1710eba195"},{"author":{"_account_id":8205,"name":"Mh Raies","email":"raiesmh08@gmail.com","username":"raiesmh08"},"change_message_id":"e61f123048506d1dec2202a59a61597cba827c54","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dFalse)"},{"line_number":35,"context_line":"        super(SecGroupRbacTest, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @classmethod"},{"line_number":38,"context_line":"    def resource_setup(cls):"},{"line_number":39,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":40,"context_line":"        secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":41,"context_line":"        cls.secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":42,"context_line":"            name\u003dsecgroup_name)[\u0027security_group\u0027]"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def resource_cleanup(cls):"},{"line_number":46,"context_line":"        # Clean up security group"},{"line_number":47,"context_line":"        test_utils.call_and_ignore_notfound_exc("},{"line_number":48,"context_line":"            cls.security_groups_client.delete_security_group,"},{"line_number":49,"context_line":"            cls.secgroup[\u0027id\u0027])"},{"line_number":50,"context_line":"        super(SecGroupRbacTest, cls).resource_cleanup()"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _create_security_group(self):"},{"line_number":53,"context_line":"        # Create a security group"}],"source_content_type":"text/x-python","patch_set":9,"id":"da36d5c6_879b6217","line":50,"range":{"start_line":37,"start_character":0,"end_line":50,"end_character":55},"in_reply_to":"da36d5c6_274b965a","updated":"2017-02-24 08:20:10.000000000","message":"Will check on this.","commit_id":"27e09d97d6a032993f79ca1800e16e1710eba195"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"338eae76be72667eafb3ba6d19acd55f84ab41d9","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        rbac_utils.switch_role(self, switchToRbacRole\u003dFalse)"},{"line_number":35,"context_line":"        super(SecGroupRbacTest, self).tearDown()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @classmethod"},{"line_number":38,"context_line":"    def resource_setup(cls):"},{"line_number":39,"context_line":"        super(SecGroupRbacTest, cls).resource_setup()"},{"line_number":40,"context_line":"        secgroup_name \u003d data_utils.rand_name(\u0027rbac-admin-secgroup\u0027)"},{"line_number":41,"context_line":"        cls.secgroup \u003d cls.security_groups_client.create_security_group("},{"line_number":42,"context_line":"            name\u003dsecgroup_name)[\u0027security_group\u0027]"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def resource_cleanup(cls):"},{"line_number":46,"context_line":"        # Clean up security group"},{"line_number":47,"context_line":"        test_utils.call_and_ignore_notfound_exc("},{"line_number":48,"context_line":"            cls.security_groups_client.delete_security_group,"},{"line_number":49,"context_line":"            cls.secgroup[\u0027id\u0027])"},{"line_number":50,"context_line":"        super(SecGroupRbacTest, cls).resource_cleanup()"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _create_security_group(self):"},{"line_number":53,"context_line":"        # Create a security group"}],"source_content_type":"text/x-python","patch_set":9,"id":"da36d5c6_7bcd1430","line":50,"range":{"start_line":37,"start_character":0,"end_line":50,"end_character":55},"in_reply_to":"da36d5c6_879b6217","updated":"2017-02-24 21:08:20.000000000","message":"ive not been successful in the past having inits on test files. let me know if that works for you.","commit_id":"27e09d97d6a032993f79ca1800e16e1710eba195"}]}