)]}'
{"etc/trove/trove-taskmanager.conf.sample":[{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"48144b1400de10d24bc691c5cffd56c596d3227a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"rabbit_password\u003df7999d1955c5014aa32c"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# Trove rollback"},{"line_number":12,"context_line":"trove_roll_back_sec_gr \u003d False"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"# SQLAlchemy connection string for the reference implementation"},{"line_number":15,"context_line":"# registry server. Any valid SQLAlchemy connection string is fine."}],"source_content_type":"application/octet-stream","patch_set":15,"id":"AAAASn%2F%2FDVg%3D","line":12,"updated":"2013-09-12 21:52:10.000000000","message":"I\u0027m not a fan of truncating variables for the sake of brevity at the cost of expressivity. It makes grep\u0027ing harder, etc. Please rename to something like sec_group_rollback_on_failure \u003d False or something more descriptive. Generally speaking, I think the \u0027trove_\u0027 prefixing should also stop, as it adds nothing of value.","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":739,"name":"Michael Basnight","email":"mbasnight@gmail.com","username":"hubcap"},"change_message_id":"291afcb8bbf77042c2a874fe773a8584c1afa133","unresolved":false,"context_lines":[{"line_number":9,"context_line":"rabbit_password\u003df7999d1955c5014aa32c"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# Trove rollback"},{"line_number":12,"context_line":"trove_roll_back_sec_gr \u003d False"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"# SQLAlchemy connection string for the reference implementation"},{"line_number":15,"context_line":"# registry server. Any valid SQLAlchemy connection string is fine."}],"source_content_type":"application/octet-stream","patch_set":15,"id":"AAAASn%2F%2FDG4%3D","line":12,"in_reply_to":"AAAASn%2F%2FDVg%3D","updated":"2013-09-12 22:46:50.000000000","message":"i TOTALLY agree with both of these. Lets rename to what auston said, sec_group_rollback_on_failure","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":8,"context_line":"# AMQP Connection info"},{"line_number":9,"context_line":"rabbit_password\u003df7999d1955c5014aa32c"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# Trove rollback"},{"line_number":12,"context_line":"sec_group_rollback_on_failure \u003d False"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"# SQLAlchemy connection string for the reference implementation"}],"source_content_type":"application/octet-stream","patch_set":21,"id":"AAAAS3%2F%2Flt8%3D","line":11,"updated":"2013-09-24 09:47:48.000000000","message":"This is not a good comment. Please can you put in something more descriptive?","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":9,"context_line":"rabbit_password\u003df7999d1955c5014aa32c"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# Trove rollback"},{"line_number":12,"context_line":"sec_group_rollback_on_failure \u003d False"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"# SQLAlchemy connection string for the reference implementation"},{"line_number":15,"context_line":"# registry server. Any valid SQLAlchemy connection string is fine."}],"source_content_type":"application/octet-stream","patch_set":21,"id":"AAAAS3%2F%2FmgY%3D","line":12,"updated":"2013-09-24 09:47:48.000000000","message":"Do we really need a conf value for this scenario?\n\nIt seems to me that we should pick a design approach and stick with it. If an instance fails to build, and we\u0027ve created a security group for it, we have two options:\n\n1. Leave it as is. When the instance is deleted / cleaned up (either by a timeout -\u003e ERROR state, or by manually resetting the state), we ensure that we then delete / clean up any associated security group and security group rules. \n\nOr, 2. Always perform a cleanup of the security group and rules on failure.\n\nAlso, I don\u0027t like naming what we are trying to do here as \"rollback\", which implies that we are reverting to a previous state. I would prefer calling it \"cleanup\" or something along those lines.","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"}],"trove/common/cfg.py":[{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"48144b1400de10d24bc691c5cffd56c596d3227a","unresolved":false,"context_lines":[{"line_number":123,"context_line":"    cfg.IntOpt(\u0027http_put_rate\u0027, default\u003d200),"},{"line_number":124,"context_line":"    cfg.BoolOpt(\u0027hostname_require_ipv4\u0027, default\u003dTrue,"},{"line_number":125,"context_line":"                help\u003d\"Require user hostnames to be IPv4 addresses.\"),"},{"line_number":126,"context_line":"    cfg.BoolOpt(\u0027trove_roll_back_sec_gr\u0027, default\u003dFalse),"},{"line_number":127,"context_line":"    cfg.BoolOpt(\u0027trove_security_groups_support\u0027, default\u003dTrue),"},{"line_number":128,"context_line":"    cfg.BoolOpt(\u0027trove_security_groups_rules_support\u0027, default\u003dTrue),"},{"line_number":129,"context_line":"    cfg.StrOpt(\u0027trove_security_group_name_prefix\u0027, default\u003d\u0027SecGroup\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDSY%3D","line":126,"updated":"2013-09-12 21:52:10.000000000","message":"Can you add \u0027help\u003d\u0027 content?","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":739,"name":"Michael Basnight","email":"mbasnight@gmail.com","username":"hubcap"},"change_message_id":"291afcb8bbf77042c2a874fe773a8584c1afa133","unresolved":false,"context_lines":[{"line_number":123,"context_line":"    cfg.IntOpt(\u0027http_put_rate\u0027, default\u003d200),"},{"line_number":124,"context_line":"    cfg.BoolOpt(\u0027hostname_require_ipv4\u0027, default\u003dTrue,"},{"line_number":125,"context_line":"                help\u003d\"Require user hostnames to be IPv4 addresses.\"),"},{"line_number":126,"context_line":"    cfg.BoolOpt(\u0027trove_roll_back_sec_gr\u0027, default\u003dFalse),"},{"line_number":127,"context_line":"    cfg.BoolOpt(\u0027trove_security_groups_support\u0027, default\u003dTrue),"},{"line_number":128,"context_line":"    cfg.BoolOpt(\u0027trove_security_groups_rules_support\u0027, default\u003dTrue),"},{"line_number":129,"context_line":"    cfg.StrOpt(\u0027trove_security_group_name_prefix\u0027, default\u003d\u0027SecGroup\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDG0%3D","line":126,"in_reply_to":"AAAASn%2F%2FDSY%3D","updated":"2013-09-12 22:46:50.000000000","message":"agreed","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"9198885d33f6bf9f8fd078a5baca1228383e1eb1","unresolved":false,"context_lines":[{"line_number":125,"context_line":"    cfg.BoolOpt(\u0027hostname_require_ipv4\u0027, default\u003dTrue,"},{"line_number":126,"context_line":"                help\u003d\"Require user hostnames to be IPv4 addresses.\"),"},{"line_number":127,"context_line":"    cfg.BoolOpt(\u0027sec_group_rollback_on_failure\u0027, default\u003dFalse,"},{"line_number":128,"context_line":"                help\u003d\u0027Flag for rolling back security group for instance \u0027"},{"line_number":129,"context_line":"                     \u0027if something goes wrong\u0027),"},{"line_number":130,"context_line":"    cfg.BoolOpt(\u0027trove_security_groups_support\u0027, default\u003dTrue),"},{"line_number":131,"context_line":"    cfg.BoolOpt(\u0027trove_security_groups_rules_support\u0027, default\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FARM%3D","line":128,"updated":"2013-09-13 17:16:49.000000000","message":"If you plan on addressing the \u0027Build error:\u0027 comment I made, you could also spruce this help content up a bit. Pointing out that it\u0027s a flag is redundant, and \u0027something goes wrong\u0027 isn\u0027t very descriptive. Perhaps:\n\n\"If trove_security_groups_support is true, deletes the orphaned security group (and associated rules) if the instance fails to build or initialize.\"\n\nGranted this is being a bit picky, but the more vigilance we use in writing good messages now, the less painful it will be in the future.","commit_id":"08aefcc7851be077b10bc96c11d2a1b8a11e410c"},{"author":{"_account_id":1925,"name":"Vipul Sabhaya","email":"vipuls@gmail.com","username":"vipuls"},"change_message_id":"38336d5ab0cc877ab1eb4451897a9428381cc7fc","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    cfg.IntOpt(\u0027http_put_rate\u0027, default\u003d200),"},{"line_number":125,"context_line":"    cfg.BoolOpt(\u0027hostname_require_ipv4\u0027, default\u003dTrue,"},{"line_number":126,"context_line":"                help\u003d\"Require user hostnames to be IPv4 addresses.\"),"},{"line_number":127,"context_line":"    cfg.BoolOpt(\u0027sec_group_rollback_on_failure\u0027, default\u003dFalse,"},{"line_number":128,"context_line":"                help\u003d\u0027If trove_security_groups_support is true, \u0027"},{"line_number":129,"context_line":"                     \u0027deletes the orphaned security group \u0027"},{"line_number":130,"context_line":"                     \u0027(and associated rules) if the instance \u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F27g%3D","line":127,"updated":"2013-09-18 06:14:25.000000000","message":"I wonder if we really need to guard this with a flag.  This is something the application should handle regardless, thus seems that a flag is unnecessary.","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"53f896ca4f0f07cced9d8656919d40b46df5e34e","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    cfg.IntOpt(\u0027http_put_rate\u0027, default\u003d200),"},{"line_number":125,"context_line":"    cfg.BoolOpt(\u0027hostname_require_ipv4\u0027, default\u003dTrue,"},{"line_number":126,"context_line":"                help\u003d\"Require user hostnames to be IPv4 addresses.\"),"},{"line_number":127,"context_line":"    cfg.BoolOpt(\u0027sec_group_rollback_on_failure\u0027, default\u003dFalse,"},{"line_number":128,"context_line":"                help\u003d\u0027If trove_security_groups_support is true, \u0027"},{"line_number":129,"context_line":"                     \u0027deletes the orphaned security group \u0027"},{"line_number":130,"context_line":"                     \u0027(and associated rules) if the instance \u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F2xI%3D","line":127,"in_reply_to":"AAAAS3%2F%2F27g%3D","updated":"2013-09-18 07:18:00.000000000","message":"maybe...","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    cfg.IntOpt(\u0027http_put_rate\u0027, default\u003d200),"},{"line_number":125,"context_line":"    cfg.BoolOpt(\u0027hostname_require_ipv4\u0027, default\u003dTrue,"},{"line_number":126,"context_line":"                help\u003d\"Require user hostnames to be IPv4 addresses.\"),"},{"line_number":127,"context_line":"    cfg.BoolOpt(\u0027sec_group_rollback_on_failure\u0027, default\u003dFalse,"},{"line_number":128,"context_line":"                help\u003d\u0027If trove_security_groups_support is true, \u0027"},{"line_number":129,"context_line":"                     \u0027deletes the orphaned security group \u0027"},{"line_number":130,"context_line":"                     \u0027(and associated rules) if the instance \u0027"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Fll8%3D","line":127,"updated":"2013-09-24 09:47:48.000000000","message":"I don\u0027t like naming what we are trying to do here as \"rollback\", which implies that we are reverting to a previous state. I would prefer calling it \"cleanup\" or something along those lines.","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"}],"trove/extensions/security_group/models.py":[{"author":{"_account_id":1925,"name":"Vipul Sabhaya","email":"vipuls@gmail.com","username":"vipuls"},"change_message_id":"db9c96e9a6ece6415be49439260035c98abe9e43","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            deleted\u003dFalse)"},{"line_number":126,"context_line":"        if association:"},{"line_number":127,"context_line":"            sec_group \u003d association.get_security_group()"},{"line_number":128,"context_line":"            RemoteSecurityGroup.delete(sec_group.id, context)"},{"line_number":129,"context_line":"            association.delete()"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Fneg%3D","line":128,"updated":"2013-09-23 21:39:40.000000000","message":"Is it necessary to change this logic?  Seems like SecurityGroup.delete() already does this and more.","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            deleted\u003dFalse)"},{"line_number":126,"context_line":"        if association:"},{"line_number":127,"context_line":"            sec_group \u003d association.get_security_group()"},{"line_number":128,"context_line":"            RemoteSecurityGroup.delete(sec_group.id, context)"},{"line_number":129,"context_line":"            association.delete()"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2FltE%3D","line":128,"updated":"2013-09-24 09:47:48.000000000","message":"This only deletes the remote group and doesn\u0027t take care of cleaning up the entry in the trove db (which will be orphaned).","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":4240,"name":"Craig Vyvial","email":"cp16net@gmail.com","username":"cp16net"},"change_message_id":"f1324fa462d24be0f1f0f17167f6344f1763595b","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                instance_id\u003dinstance_id,"},{"line_number":126,"context_line":"                deleted\u003dFalse)"},{"line_number":127,"context_line":"            if association:"},{"line_number":128,"context_line":"                    sec_group \u003d association.get_security_group()"},{"line_number":129,"context_line":"                    if sec_group:"},{"line_number":130,"context_line":"                        sec_group.delete(context)"},{"line_number":131,"context_line":"                    association.delete()"}],"source_content_type":"text/x-python","patch_set":38,"id":"AAAATH%2F%2Ftls%3D","line":128,"updated":"2013-10-10 20:48:04.000000000","message":"please make this indent 4 spaces instead of 8?","commit_id":"4781b75267fe418569654fd1bb89e8cab8e12dfd"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"4c6c48dbc91a8dd245d1497cc117a565b86e26ec","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                instance_id\u003dinstance_id,"},{"line_number":126,"context_line":"                deleted\u003dFalse)"},{"line_number":127,"context_line":"            if association:"},{"line_number":128,"context_line":"                    sec_group \u003d association.get_security_group()"},{"line_number":129,"context_line":"                    if sec_group:"},{"line_number":130,"context_line":"                        sec_group.delete(context)"},{"line_number":131,"context_line":"                    association.delete()"}],"source_content_type":"text/x-python","patch_set":38,"id":"AAAATH%2F%2FtkY%3D","line":128,"in_reply_to":"AAAATH%2F%2Ftls%3D","updated":"2013-10-10 20:50:15.000000000","message":"Oh, yes, i\u0027ll do that, but currently it would not cause any style error. This code is friendly with pep8 checks.","commit_id":"4781b75267fe418569654fd1bb89e8cab8e12dfd"}],"trove/instance/models.py":[{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"48144b1400de10d24bc691c5cffd56c596d3227a","unresolved":false,"context_lines":[{"line_number":489,"context_line":"                        InstanceTasks.BUILDING_ERROR_SEC_GROUP"},{"line_number":490,"context_line":"                    db_info.save()"},{"line_number":491,"context_line":"                    service_status.set_status(ServiceStatuses.FAILED)"},{"line_number":492,"context_line":"                    service_status.save()"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"            task_api.API(context).create_instance(db_info.id, name, flavor,"},{"line_number":495,"context_line":"                                                  image_id, databases, users,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDRA%3D","line":492,"updated":"2013-09-12 21:52:10.000000000","message":"Shouldn\u0027t you re-raise here?","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":739,"name":"Michael Basnight","email":"mbasnight@gmail.com","username":"hubcap"},"change_message_id":"291afcb8bbf77042c2a874fe773a8584c1afa133","unresolved":false,"context_lines":[{"line_number":489,"context_line":"                        InstanceTasks.BUILDING_ERROR_SEC_GROUP"},{"line_number":490,"context_line":"                    db_info.save()"},{"line_number":491,"context_line":"                    service_status.set_status(ServiceStatuses.FAILED)"},{"line_number":492,"context_line":"                    service_status.save()"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"            task_api.API(context).create_instance(db_info.id, name, flavor,"},{"line_number":495,"context_line":"                                                  image_id, databases, users,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDGk%3D","line":492,"in_reply_to":"AAAASn%2F%2FDQM%3D","updated":"2013-09-12 22:46:50.000000000","message":"We should. Otherwise we risk calling the taskmanager to create the instance, making it more work to clean up later.","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"427c6a31a303c13ac2a6e805445c2c4bd25c275f","unresolved":false,"context_lines":[{"line_number":489,"context_line":"                        InstanceTasks.BUILDING_ERROR_SEC_GROUP"},{"line_number":490,"context_line":"                    db_info.save()"},{"line_number":491,"context_line":"                    service_status.set_status(ServiceStatuses.FAILED)"},{"line_number":492,"context_line":"                    service_status.save()"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"            task_api.API(context).create_instance(db_info.id, name, flavor,"},{"line_number":495,"context_line":"                                                  image_id, databases, users,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDQM%3D","line":492,"in_reply_to":"AAAASn%2F%2FDRA%3D","updated":"2013-09-12 21:59:49.000000000","message":"Yes, we could","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":1375,"name":"Ed Cranford","email":"ed.cranford@gmail.com","username":"ed--cranford"},"change_message_id":"bf665dc7399c4495cbe9f976ae6b6756f14a09ec","unresolved":false,"context_lines":[{"line_number":487,"context_line":"                    security_groups \u003d [security_group[\"name\"]]"},{"line_number":488,"context_line":"                except (exception.SecurityGroupCreationError,"},{"line_number":489,"context_line":"                        exception.SecurityGroupRuleCreationError) as ex:"},{"line_number":490,"context_line":"                    db_info.task_status \u003d \\"},{"line_number":491,"context_line":"                        InstanceTasks.BUILDING_ERROR_SEC_GROUP"},{"line_number":492,"context_line":"                    db_info.save()"},{"line_number":493,"context_line":"                    service_status.set_status(ServiceStatuses.FAILED)"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAS3%2F%2F9AY%3D","line":490,"updated":"2013-09-16 17:02:32.000000000","message":"\\","commit_id":"995c567c6eb00ce250ea8c831ebb976dc801131a"},{"author":{"_account_id":1375,"name":"Ed Cranford","email":"ed.cranford@gmail.com","username":"ed--cranford"},"change_message_id":"b594af5136a9e021fc1db1bf56e6f930f10d1333","unresolved":false,"context_lines":[{"line_number":487,"context_line":"                    security_groups \u003d [security_group[\"name\"]]"},{"line_number":488,"context_line":"                except (exception.SecurityGroupCreationError,"},{"line_number":489,"context_line":"                        exception.SecurityGroupRuleCreationError) as ex:"},{"line_number":490,"context_line":"                    db_info.task_status \u003d \\"},{"line_number":491,"context_line":"                        InstanceTasks.BUILDING_ERROR_SEC_GROUP"},{"line_number":492,"context_line":"                    db_info.save()"},{"line_number":493,"context_line":"                    service_status.set_status(ServiceStatuses.FAILED)"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAS3%2F%2F8wo%3D","line":490,"updated":"2013-09-16 18:03:50.000000000","message":"how about\n    besg \u003d InstanceTasks.BUILDING_ERROR_SEC_GROUP\n    db_info.task_status \u003d besg","commit_id":"995c567c6eb00ce250ea8c831ebb976dc801131a"},{"author":{"_account_id":7593,"name":"Haomai Wang","email":"haomaiwang@gmail.com","username":"HaomaiWang"},"change_message_id":"1d1aab76426395b9fa22475b80edcea6c318bd18","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                                           BUILDING_ERROR_SEC_GROUP)"},{"line_number":494,"context_line":"                    db_info.save()"},{"line_number":495,"context_line":"                    service_status.set_status(ServiceStatuses.FAILED)"},{"line_number":496,"context_line":"                    service_status.save()"},{"line_number":497,"context_line":"                    raise ex"},{"line_number":498,"context_line":"            task_api.API(context).create_instance(db_info.id, name, flavor,"},{"line_number":499,"context_line":"                                                  image_id, databases, users,"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F21k%3D","line":496,"updated":"2013-09-18 06:53:59.000000000","message":"log.Exception()?","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":497,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":498,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":499,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"},{"line_number":500,"context_line":"                    raise ex"},{"line_number":501,"context_line":"            task_api.API(context).create_instance(db_info.id, name, flavor,"},{"line_number":502,"context_line":"                                                  image_id, databases, users,"},{"line_number":503,"context_line":"                                                  service_type, volume_size,"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Flsc%3D","line":500,"updated":"2013-09-24 09:47:48.000000000","message":"Do not re-raise the exception explicitly. This will get rid of the old stack trace, since the new exception will be raised from here. Just use \"raise\" instead, which will re-raise the last exception and preserve the traceback.","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"6970c769cce7b552ce76cd66c58687fc9a6b4e05","unresolved":false,"context_lines":[{"line_number":491,"context_line":"                    security_groups \u003d [security_group[\"name\"]]"},{"line_number":492,"context_line":"                except (exception.SecurityGroupCreationError,"},{"line_number":493,"context_line":"                        exception.SecurityGroupRuleCreationError):"},{"line_number":494,"context_line":"                    db_info.task_status \u003d (InstanceTasks."},{"line_number":495,"context_line":"                                           BUILDING_ERROR_SEC_GROUP)"},{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FLsQ%3D","line":494,"updated":"2013-10-02 19:30:55.000000000","message":"The DB record has to be marked DELETED or be deleted because this exception will cause quota to rollback to its original usage. So if the record remains there and the user tries to delete it after, the quota usage will falsely decrease.","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"a9dc95ccc8ee7e11f227785f28f48593d40da997","unresolved":false,"context_lines":[{"line_number":491,"context_line":"                    security_groups \u003d [security_group[\"name\"]]"},{"line_number":492,"context_line":"                except (exception.SecurityGroupCreationError,"},{"line_number":493,"context_line":"                        exception.SecurityGroupRuleCreationError):"},{"line_number":494,"context_line":"                    db_info.task_status \u003d (InstanceTasks."},{"line_number":495,"context_line":"                                           BUILDING_ERROR_SEC_GROUP)"},{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FLlY%3D","line":494,"in_reply_to":"AAAAS3%2F%2FLlk%3D","updated":"2013-10-02 20:04:46.000000000","message":"we can mark the instance as Failed but then you would have to swallow the exception. If you re-raise it, the quota engine will rollback the quota.","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"87beaabb666d22b3c44ab0f9382e6febfea7e688","unresolved":false,"context_lines":[{"line_number":491,"context_line":"                    security_groups \u003d [security_group[\"name\"]]"},{"line_number":492,"context_line":"                except (exception.SecurityGroupCreationError,"},{"line_number":493,"context_line":"                        exception.SecurityGroupRuleCreationError):"},{"line_number":494,"context_line":"                    db_info.task_status \u003d (InstanceTasks."},{"line_number":495,"context_line":"                                           BUILDING_ERROR_SEC_GROUP)"},{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FLlk%3D","line":494,"in_reply_to":"AAAAS3%2F%2FLsQ%3D","updated":"2013-10-02 20:02:58.000000000","message":"But when instance failed, user could delete instance and than quota would be updated.","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"f873d2eb0372ca7396249232832acf521a365d0c","unresolved":false,"context_lines":[{"line_number":491,"context_line":"                    security_groups \u003d [security_group[\"name\"]]"},{"line_number":492,"context_line":"                except (exception.SecurityGroupCreationError,"},{"line_number":493,"context_line":"                        exception.SecurityGroupRuleCreationError):"},{"line_number":494,"context_line":"                    db_info.task_status \u003d (InstanceTasks."},{"line_number":495,"context_line":"                                           BUILDING_ERROR_SEC_GROUP)"},{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FLq0%3D","line":494,"in_reply_to":"AAAAS3%2F%2FLsQ%3D","updated":"2013-10-02 19:34:57.000000000","message":"good point","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"aae37d9c9aa3e3a1c551b8cfd1a0b214fbf441a6","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    db_info.delete()"},{"line_number":498,"context_line":"                    service_status.set_status("},{"line_number":499,"context_line":"                        rd_instance.ServiceStatuses.DELETED)"},{"line_number":500,"context_line":"                    service_status.save()"},{"line_number":501,"context_line":"                    service_status.delete()"},{"line_number":502,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""}],"source_content_type":"text/x-python","patch_set":31,"id":"AAAAS3%2F%2FIeY%3D","line":499,"updated":"2013-10-03 18:58:56.000000000","message":"The status should be FAILED. We dont want to mark it as DELETED because we dont want to rollback. This is why we are swallowing the exception. So that the quota usage gets incremented.\n\nWhen the user sees that failed instance, he/she will delete it, which will lower the quota.","commit_id":"bfa31b7f05134cf1ccfa7d3a262ce1c115ce670a"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"de2395fd64a02fe144c7fcf419e1d5f1b466cdd3","unresolved":false,"context_lines":[{"line_number":494,"context_line":"                    db_info.task_status \u003d (InstanceTasks."},{"line_number":495,"context_line":"                                           BUILDING_ERROR_SEC_GROUP)"},{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    db_info.delete()"},{"line_number":498,"context_line":"                    service_status.set_status("},{"line_number":499,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":500,"context_line":"                    service_status.save()"}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FFZ4%3D","line":497,"updated":"2013-10-04 17:07:45.000000000","message":"Remove the db_info.delete() line. Even though the instance failed, we dont want it to be removed. We want to show the user that it failed.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":739,"name":"Michael Basnight","email":"mbasnight@gmail.com","username":"hubcap"},"change_message_id":"ad41bd437e4a1ec718069516fce5bad0e2785499","unresolved":false,"context_lines":[{"line_number":494,"context_line":"                    db_info.task_status \u003d (InstanceTasks."},{"line_number":495,"context_line":"                                           BUILDING_ERROR_SEC_GROUP)"},{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    db_info.delete()"},{"line_number":498,"context_line":"                    service_status.set_status("},{"line_number":499,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":500,"context_line":"                    service_status.save()"}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FFIY%3D","line":497,"in_reply_to":"AAAAS3%2F%2FFZ4%3D","updated":"2013-10-04 18:28:54.000000000","message":"agreed.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"de2395fd64a02fe144c7fcf419e1d5f1b466cdd3","unresolved":false,"context_lines":[{"line_number":498,"context_line":"                    service_status.set_status("},{"line_number":499,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":500,"context_line":"                    service_status.save()"},{"line_number":501,"context_line":"                    service_status.delete()"},{"line_number":502,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":503,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":504,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FFZk%3D","line":501,"updated":"2013-10-04 17:07:45.000000000","message":"Same thing with this line. Dont mark it as deleted","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":694,"name":"Tim Simpson","email":"tim.simpson@rackspace.com","username":"tim-simpson"},"change_message_id":"4ba85258df0cfba857aff1b9f5789ae0f0fbb269","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F4Ag%3D","line":499,"updated":"2013-10-08 16:06:45.000000000","message":"I really like the added code to set the task status to indicate precisely what went wrong. That will help out operators as they will be able to see exactly what\u0027s wrong using the MGMT instance call and not have to search the logs.\n\nHowever, from the moment the status code is set, the API users will see \"ERROR\" as the status for the instance (because \"is_error\u003dTrue\" for the new BUILDING_ERROR_SEC_GROUP). So they won\u0027t trust using the instance even if it provisions perfectly except for the security groups. Because there is no MGMT API call to add a security group to an instance and fix it it also means nothing can be done to fix the instance.\n\nBecause the instance is effectively useless at this point, on line 503 I think you should add a call to re-raise the exception and stop this code path, instead of allowing the task manager to continue the provisioning process.","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"58c6d811348ba292ca36ab709020e54917952301","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F13w%3D","line":499,"in_reply_to":"AAAATH%2F%2F2pM%3D","updated":"2013-10-09 04:42:50.000000000","message":"I really don\u0027t like the idea of moving this code to taskmanager. As we discussed yesterday, we suppose to create this kind of components after trove instance would become active, but(!!!) according neares future update, security group would a default part of heat template, so, it would not be possible to use nova calls after heat provisioning.\n\nSo, my suggestion is next. Let\u0027s add finally block at line 503, which would check if security groups variable still equals None(means that something went wrong while creation),\nif it so - retrun None.","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"70a7e58c4f37da548e71a259c1efb6833c3ebf5a","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F2pM%3D","line":499,"in_reply_to":"AAAATH%2F%2F2q4%3D","updated":"2013-10-08 23:10:40.000000000","message":"Got confused a bit. Yes Tim. That would work. Moving that block of code to the TM and adding a raise when the error happens. The error is handled in the TM and quota would not be affected","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"35d9d9fc8412340971d33330b4322042da45a31b","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F2q4%3D","line":499,"in_reply_to":"AAAATH%2F%2F2tM%3D","updated":"2013-10-08 23:01:25.000000000","message":"It would still be an issue. The real problem is the DB record created in line 458. Whether we raise or not raise, this instance will still be shown to the user. We have two \u0027easy\u0027 ways to solve this: \n1- We raise and mark the instance \u0027deleted\u0027. Quota rolls back to whatever it was before. All user will see is the error message.\n\n2- We dont raise but we mark the instance as \u0027Failed\u0027. Quota consumes a resource, which will be freed up once the user deletes the failed instance.\n\nHubcap didn\u0027t like option 1. So we settled with option 2.\n\nI share your concern about the call to taskmanager when the instance has already failed. I was suggesting to add some logic around the taskmanager-api call (line 504) to execute if there are no errors.","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":694,"name":"Tim Simpson","email":"tim.simpson@rackspace.com","username":"tim-simpson"},"change_message_id":"1bf8c49258d1bf8fef3b05bfd95ceaf8f4e14cf7","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F2tM%3D","line":499,"in_reply_to":"AAAATH%2F%2F2u4%3D","updated":"2013-10-08 22:52:29.000000000","message":"Hi Steve,\n\nWhat if the raise *was* added, and it along with lines 477-502 moved into the task manager? Would the quotas feature work then?","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"725cf3edc8d019b6b44cb826c8586da4eb3e7eb1","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F3YE%3D","line":499,"in_reply_to":"AAAATH%2F%2F304%3D","updated":"2013-10-08 19:26:41.000000000","message":"I understand Tim\u0027s concern but re-raising the error will cause the quota to be rolled back. We dont want that because the failed instance will still show in the DB but will not consume any quota.\n\nPerhaps we could return in the \u0027except\u0027 block. Or have a log around that taskmanager call to execute if no errors were encountered. But we cant reraise. If we decide we want to re-raise, we will need to roll back the DB record created for that instance (so it doesnt show to the user.)","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"f79a458b0c6e272a3a5649cc438a11ea331e7cc8","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F2u4%3D","line":499,"in_reply_to":"AAAATH%2F%2F3Ug%3D","updated":"2013-10-08 22:39:53.000000000","message":"This cannot be abandoned because this bug still affects quota. Which core-dev decided this?","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"8103917f7590241c8d855fe5197e53aaecea2dca","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F3Ug%3D","line":499,"in_reply_to":"AAAATH%2F%2F3YE%3D","updated":"2013-10-08 19:40:16.000000000","message":"Unfortunately, this is a time-stub until trove gets v2 API. So this review is going to be closed. Cores decided not to do anything with this kind of resources. So, main goal is to update instance task status and service status if instance would stuck at BUILDING status https://review.openstack.org/#/c/45723/","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"68b51ef8a222df29750a31b76a13c81a36c73635","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    db_info.save()"},{"line_number":497,"context_line":"                    service_status.set_status("},{"line_number":498,"context_line":"                        rd_instance.ServiceStatuses.FAILED)"},{"line_number":499,"context_line":"                    service_status.save()"},{"line_number":500,"context_line":"                    LOG.error(_(\"Cannot create and associate security group:\""},{"line_number":501,"context_line":"                                \" for instance: %(inst_id)s\")"},{"line_number":502,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"}],"source_content_type":"text/x-python","patch_set":36,"id":"AAAATH%2F%2F304%3D","line":499,"in_reply_to":"AAAATH%2F%2F4Ag%3D","updated":"2013-10-08 19:07:55.000000000","message":"Security group is optional, so when nova provising instance by default it has a default security group. So, somehow i agree with re-raising to rollback quota.","commit_id":"5454a785f7d65a311bb7798f7b9cfa84f1bb28c6"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"861f6aadfc69dc9af9c17106f9fe8a7cbd0921a0","unresolved":false,"context_lines":[{"line_number":503,"context_line":"                    raise"},{"line_number":504,"context_line":"                finally:"},{"line_number":505,"context_line":"                    if not security_groups:"},{"line_number":506,"context_line":"                        return"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"            task_api.API(context).create_instance(db_info.id, name, flavor,"},{"line_number":509,"context_line":"                                                  image_id, databases, users,"}],"source_content_type":"text/x-python","patch_set":37,"id":"AAAATH%2F%2FzJI%3D","line":506,"updated":"2013-10-09 17:07:59.000000000","message":"I think you need to return an instance of \"SimpleInstance\" since it is being expected in the InstanceView. So perhaps instead of \u0027return\u0027 have a  \nreturn SimpleInstance(context, db_info, service_status)","commit_id":"c336afeb1f142e2b54277091f8deaea89392c419"},{"author":{"_account_id":694,"name":"Tim Simpson","email":"tim.simpson@rackspace.com","username":"tim-simpson"},"change_message_id":"c3f6d2383ea671bb44a60e9412bb8f282f3ca008","unresolved":false,"context_lines":[{"line_number":508,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"},{"line_number":509,"context_line":"                    raise"},{"line_number":510,"context_line":"                finally:"},{"line_number":511,"context_line":"                    if not security_groups:"},{"line_number":512,"context_line":"                        return SimpleInstance(context, db_info, service_status)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"            root_password \u003d None"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAATH%2F%2FYe0%3D","line":511,"updated":"2013-10-16 21:16:09.000000000","message":"Won\u0027t this be an attribute error if the lines 485-495 hit an exception? In that case, security_groups will never be set, and line 511 will be an attribute error. Of course, the exception will still be raise, but it seems like line 511-512 should live within the if statement on line 483 but outside the try / except block.","commit_id":"ee9d3dd5b116a460be5a01e27e5e63755303edaa"},{"author":{"_account_id":694,"name":"Tim Simpson","email":"tim.simpson@rackspace.com","username":"tim-simpson"},"change_message_id":"f3f21c993d60b7104b2e839eb5ded80401c2ee53","unresolved":false,"context_lines":[{"line_number":508,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"},{"line_number":509,"context_line":"                    raise"},{"line_number":510,"context_line":"                finally:"},{"line_number":511,"context_line":"                    if not security_groups:"},{"line_number":512,"context_line":"                        return SimpleInstance(context, db_info, service_status)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"            root_password \u003d None"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAATH%2F%2FYXk%3D","line":511,"in_reply_to":"AAAATH%2F%2FYZk%3D","updated":"2013-10-16 22:43:20.000000000","message":"Sorry, I misread line 511 as \"if not security_group:\". Oops.","commit_id":"ee9d3dd5b116a460be5a01e27e5e63755303edaa"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"ae5da13220dbfc32018ae64424fce46609b41998","unresolved":false,"context_lines":[{"line_number":508,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"},{"line_number":509,"context_line":"                    raise"},{"line_number":510,"context_line":"                finally:"},{"line_number":511,"context_line":"                    if not security_groups:"},{"line_number":512,"context_line":"                        return SimpleInstance(context, db_info, service_status)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"            root_password \u003d None"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAATH%2F%2FYVg%3D","line":511,"in_reply_to":"AAAATH%2F%2FYe0%3D","updated":"2013-10-16 21:50:02.000000000","message":"When the exception hits, the value of security_groups is None. And therefore, the condition will pass and it will just return the SimpleInstance object","commit_id":"ee9d3dd5b116a460be5a01e27e5e63755303edaa"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"ebf3e10bf3449dfb69c2ddadf9ccbad8c455d2fc","unresolved":false,"context_lines":[{"line_number":508,"context_line":"                              % {\u0027inst_id\u0027: db_info.id})"},{"line_number":509,"context_line":"                    raise"},{"line_number":510,"context_line":"                finally:"},{"line_number":511,"context_line":"                    if not security_groups:"},{"line_number":512,"context_line":"                        return SimpleInstance(context, db_info, service_status)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"            root_password \u003d None"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAATH%2F%2FYZk%3D","line":511,"in_reply_to":"AAAATH%2F%2FYe0%3D","updated":"2013-10-16 21:27:12.000000000","message":"it should not hit attribute error, because security groups are optional.\nso, i suggest to make a double-check:\nif not security_groups and CONF.trove_security_groups_support:\nreturn SimpleInstance(context, db_info, service_status)","commit_id":"ee9d3dd5b116a460be5a01e27e5e63755303edaa"},{"author":{"_account_id":694,"name":"Tim Simpson","email":"tim.simpson@rackspace.com","username":"tim-simpson"},"change_message_id":"c3f6d2383ea671bb44a60e9412bb8f282f3ca008","unresolved":false,"context_lines":[{"line_number":509,"context_line":"                    raise"},{"line_number":510,"context_line":"                finally:"},{"line_number":511,"context_line":"                    if not security_groups:"},{"line_number":512,"context_line":"                        return SimpleInstance(context, db_info, service_status)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"            root_password \u003d None"},{"line_number":515,"context_line":"            if CONF.root_on_create and not backup_id:"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAATH%2F%2FYeE%3D","line":512,"updated":"2013-10-16 21:16:09.000000000","message":"If SimpleInstance is returned, then won\u0027t the instance never begin building? Also, it seems like if \"security_groups\" is None, there\u0027s no point in continuing and an exception should be raised. Though maybe I\u0027m misunderstanding.","commit_id":"ee9d3dd5b116a460be5a01e27e5e63755303edaa"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"ae5da13220dbfc32018ae64424fce46609b41998","unresolved":false,"context_lines":[{"line_number":509,"context_line":"                    raise"},{"line_number":510,"context_line":"                finally:"},{"line_number":511,"context_line":"                    if not security_groups:"},{"line_number":512,"context_line":"                        return SimpleInstance(context, db_info, service_status)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"            root_password \u003d None"},{"line_number":515,"context_line":"            if CONF.root_on_create and not backup_id:"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAATH%2F%2FYVE%3D","line":512,"in_reply_to":"AAAATH%2F%2FYeE%3D","updated":"2013-10-16 21:50:02.000000000","message":"Returning a SimpleInstance object will prevent the instance from building. Which is want we want since secgroup provisionining failed. \n\nThe reason why we need to return a SImpleInstance, it is because it is expected in the view. \n\nAn error should not be raised because this will cause the quota to rollback. Doing this the quota will rollback but the \u0027instance\u0027 will still exist","commit_id":"ee9d3dd5b116a460be5a01e27e5e63755303edaa"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"ebf3e10bf3449dfb69c2ddadf9ccbad8c455d2fc","unresolved":false,"context_lines":[{"line_number":509,"context_line":"                    raise"},{"line_number":510,"context_line":"                finally:"},{"line_number":511,"context_line":"                    if not security_groups:"},{"line_number":512,"context_line":"                        return SimpleInstance(context, db_info, service_status)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"            root_password \u003d None"},{"line_number":515,"context_line":"            if CONF.root_on_create and not backup_id:"}],"source_content_type":"text/x-python","patch_set":39,"id":"AAAATH%2F%2FYZY%3D","line":512,"in_reply_to":"AAAATH%2F%2FYeE%3D","updated":"2013-10-16 21:27:12.000000000","message":"this case would be covered by previous comment","commit_id":"ee9d3dd5b116a460be5a01e27e5e63755303edaa"}],"trove/instance/tasks.py":[{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"48144b1400de10d24bc691c5cffd56c596d3227a","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    BUILDING_ERROR_VOLUME \u003d InstanceTask(0x52, \u0027BUILDING\u0027,"},{"line_number":80,"context_line":"                                         \u0027Build error: Volume.\u0027,"},{"line_number":81,"context_line":"                                         is_error\u003dTrue)"},{"line_number":82,"context_line":"    BUILDING_ERROR_SEC_GROUP \u003d InstanceTask(0x53, \u0027ERROR\u0027,"},{"line_number":83,"context_line":"                                            \u0027Security group/rule exception\u0027,"},{"line_number":84,"context_line":"                                            is_error\u003dTrue)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDRI%3D","line":82,"updated":"2013-09-12 21:52:10.000000000","message":"Notice above that the others have \u0027BUILDING\u0027, but a message describing \u0027Build error:\u0027. Since yours is also a build error, shouldn\u0027t you follow the same pattern?","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":739,"name":"Michael Basnight","email":"mbasnight@gmail.com","username":"hubcap"},"change_message_id":"291afcb8bbf77042c2a874fe773a8584c1afa133","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    BUILDING_ERROR_VOLUME \u003d InstanceTask(0x52, \u0027BUILDING\u0027,"},{"line_number":80,"context_line":"                                         \u0027Build error: Volume.\u0027,"},{"line_number":81,"context_line":"                                         is_error\u003dTrue)"},{"line_number":82,"context_line":"    BUILDING_ERROR_SEC_GROUP \u003d InstanceTask(0x53, \u0027ERROR\u0027,"},{"line_number":83,"context_line":"                                            \u0027Security group/rule exception\u0027,"},{"line_number":84,"context_line":"                                            is_error\u003dTrue)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDGc%3D","line":82,"in_reply_to":"AAAASn%2F%2FDRI%3D","updated":"2013-09-12 22:46:50.000000000","message":"agreed.","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"9198885d33f6bf9f8fd078a5baca1228383e1eb1","unresolved":false,"context_lines":[{"line_number":80,"context_line":"                                         \u0027Build error: Volume.\u0027,"},{"line_number":81,"context_line":"                                         is_error\u003dTrue)"},{"line_number":82,"context_line":"    BUILDING_ERROR_SEC_GROUP \u003d InstanceTask(0x53, \u0027BUILDING\u0027,"},{"line_number":83,"context_line":"                                            \u0027Security group/rule exception\u0027,"},{"line_number":84,"context_line":"                                            is_error\u003dTrue)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FAS4%3D","line":83,"updated":"2013-09-13 17:16:49.000000000","message":"For consistency, shouldn\u0027t this be \"Build error: Security group/rule.\"? (You\u0027re missing the \u0027Build error\u0027 prefix plus you added on \u0027exception\u0027).","commit_id":"08aefcc7851be077b10bc96c11d2a1b8a11e410c"},{"author":{"_account_id":1925,"name":"Vipul Sabhaya","email":"vipuls@gmail.com","username":"vipuls"},"change_message_id":"38336d5ab0cc877ab1eb4451897a9428381cc7fc","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                                         is_error\u003dTrue)"},{"line_number":82,"context_line":"    BUILDING_ERROR_SEC_GROUP \u003d InstanceTask(0x53, \u0027BUILDING\u0027,"},{"line_number":83,"context_line":"                                            \u0027Build error: Security group\u0027"},{"line_number":84,"context_line":"                                            \u0027or rule exception\u0027,"},{"line_number":85,"context_line":"                                            is_error\u003dTrue)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F27s%3D","line":84,"updated":"2013-09-18 06:14:25.000000000","message":"Why do we need the word \u0027exception\u0027?","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":80,"context_line":"                                         \u0027Build error: Volume.\u0027,"},{"line_number":81,"context_line":"                                         is_error\u003dTrue)"},{"line_number":82,"context_line":"    BUILDING_ERROR_SEC_GROUP \u003d InstanceTask(0x53, \u0027BUILDING\u0027,"},{"line_number":83,"context_line":"                                            \u0027Build error: Security group\u0027"},{"line_number":84,"context_line":"                                            \u0027or rule\u0027,"},{"line_number":85,"context_line":"                                            is_error\u003dTrue)"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2FlsM%3D","line":83,"updated":"2013-09-24 09:47:48.000000000","message":"Missing a space in your multi-line string.","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":7593,"name":"Haomai Wang","email":"haomaiwang@gmail.com","username":"HaomaiWang"},"change_message_id":"6d005ed0442aaa037e0117d1e5fd7996304ecfae","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                                         is_error\u003dTrue)"},{"line_number":82,"context_line":"    BUILDING_ERROR_SEC_GROUP \u003d InstanceTask(0x53, \u0027BUILDING\u0027,"},{"line_number":83,"context_line":"                                            \u0027Build error: Security group \u0027"},{"line_number":84,"context_line":"                                            \u0027or rule\u0027,"},{"line_number":85,"context_line":"                                            is_error\u003dTrue)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FJ2U%3D","line":84,"updated":"2013-10-03 12:17:22.000000000","message":"missing period \".\"","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"}],"trove/taskmanager/models.py":[{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"48144b1400de10d24bc691c5cffd56c596d3227a","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            self.send_usage_event(\u0027create\u0027, instance_size\u003dflavor[\u0027ram\u0027])"},{"line_number":190,"context_line":"        except PollTimeOut:"},{"line_number":191,"context_line":"            if CONF.trove_roll_back_sec_gr:"},{"line_number":192,"context_line":"                if server and security_groups:"},{"line_number":193,"context_line":"                    for security_group in security_groups:"},{"line_number":194,"context_line":"                        self._nova_client.servers. \\"},{"line_number":195,"context_line":"                            remove_security_group(server,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDR8%3D","line":192,"updated":"2013-09-12 21:52:10.000000000","message":"why the check for \u0027server\u0027 here?","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"60269a81ecf6297e469ef7ac1658dcb7c16f9b22","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            self.send_usage_event(\u0027create\u0027, instance_size\u003dflavor[\u0027ram\u0027])"},{"line_number":190,"context_line":"        except PollTimeOut:"},{"line_number":191,"context_line":"            if CONF.trove_roll_back_sec_gr:"},{"line_number":192,"context_line":"                if server and security_groups:"},{"line_number":193,"context_line":"                    for security_group in security_groups:"},{"line_number":194,"context_line":"                        self._nova_client.servers. \\"},{"line_number":195,"context_line":"                            remove_security_group(server,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDNw%3D","line":192,"in_reply_to":"AAAASn%2F%2FDPs%3D","updated":"2013-09-12 22:02:34.000000000","message":"makes sense.","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"427c6a31a303c13ac2a6e805445c2c4bd25c275f","unresolved":false,"context_lines":[{"line_number":189,"context_line":"            self.send_usage_event(\u0027create\u0027, instance_size\u003dflavor[\u0027ram\u0027])"},{"line_number":190,"context_line":"        except PollTimeOut:"},{"line_number":191,"context_line":"            if CONF.trove_roll_back_sec_gr:"},{"line_number":192,"context_line":"                if server and security_groups:"},{"line_number":193,"context_line":"                    for security_group in security_groups:"},{"line_number":194,"context_line":"                        self._nova_client.servers. \\"},{"line_number":195,"context_line":"                            remove_security_group(server,"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDPs%3D","line":192,"in_reply_to":"AAAASn%2F%2FDR8%3D","updated":"2013-09-12 21:59:49.000000000","message":"because of absents of ability to delete security group attached to instance, if no server check we could catch punch in a face with exception while deleting security group","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"48144b1400de10d24bc691c5cffd56c596d3227a","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                if server and security_groups:"},{"line_number":193,"context_line":"                    for security_group in security_groups:"},{"line_number":194,"context_line":"                        self._nova_client.servers. \\"},{"line_number":195,"context_line":"                            remove_security_group(server,"},{"line_number":196,"context_line":"                                                  security_group[\"name\"])"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDRc%3D","line":195,"updated":"2013-09-12 21:52:10.000000000","message":"Can you quickly clarify whether this removes the security group from the instance, or whether it deletes the security group completely? Is the server_id used to verify the association (to avoid accidentally deleting a security group by the name of \"abc\" that isn\u0027t even associated to server_id 1234)?","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"60269a81ecf6297e469ef7ac1658dcb7c16f9b22","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                if server and security_groups:"},{"line_number":193,"context_line":"                    for security_group in security_groups:"},{"line_number":194,"context_line":"                        self._nova_client.servers. \\"},{"line_number":195,"context_line":"                            remove_security_group(server,"},{"line_number":196,"context_line":"                                                  security_group[\"name\"])"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDN0%3D","line":195,"in_reply_to":"AAAASn%2F%2FDPE%3D","updated":"2013-09-12 22:02:34.000000000","message":"cool, thanks for verifying.","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"427c6a31a303c13ac2a6e805445c2c4bd25c275f","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                if server and security_groups:"},{"line_number":193,"context_line":"                    for security_group in security_groups:"},{"line_number":194,"context_line":"                        self._nova_client.servers. \\"},{"line_number":195,"context_line":"                            remove_security_group(server,"},{"line_number":196,"context_line":"                                                  security_group[\"name\"])"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAASn%2F%2FDPE%3D","line":195,"in_reply_to":"AAAASn%2F%2FDRc%3D","updated":"2013-09-12 21:59:49.000000000","message":"this part of code will delete only security group assigned to instance, or we get the exception with removing issue in it.","commit_id":"171a5439b95442e69d3679b26ac1d689204bb414"},{"author":{"_account_id":1375,"name":"Ed Cranford","email":"ed.cranford@gmail.com","username":"ed--cranford"},"change_message_id":"bf665dc7399c4495cbe9f976ae6b6756f14a09ec","unresolved":false,"context_lines":[{"line_number":35,"context_line":"from swiftclient.client import ClientException"},{"line_number":36,"context_line":"from trove.common.utils import poll_until"},{"line_number":37,"context_line":"from trove.instance import models as inst_models"},{"line_number":38,"context_line":"from trove.instance.models import BuiltInstance, DBInstance"},{"line_number":39,"context_line":"from trove.instance.models import FreshInstance"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"from trove.instance.models import InstanceStatus"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAS3%2F%2F9CI%3D","line":38,"updated":"2013-09-16 17:02:32.000000000","message":"One import per line please","commit_id":"995c567c6eb00ce250ea8c831ebb976dc801131a"},{"author":{"_account_id":1375,"name":"Ed Cranford","email":"ed.cranford@gmail.com","username":"ed--cranford"},"change_message_id":"bf665dc7399c4495cbe9f976ae6b6756f14a09ec","unresolved":false,"context_lines":[{"line_number":700,"context_line":"                if name:"},{"line_number":701,"context_line":"                    LOG.info(\"Deleting file: %s/%s\", cont, name)"},{"line_number":702,"context_line":"                    client.delete_object(cont, name)"},{"line_number":703,"context_line":"                    # Delete the manifest file"},{"line_number":704,"context_line":"        LOG.info(\"Deleting file: %s/%s\", container, filename)"},{"line_number":705,"context_line":"        client.delete_object(container, filename)"},{"line_number":706,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAS3%2F%2F9Bw%3D","line":703,"updated":"2013-09-16 17:02:32.000000000","message":"I think this line should be indented to match 704, but really since the LOG.info line says the same thing we might as well just remove the comment.","commit_id":"995c567c6eb00ce250ea8c831ebb976dc801131a"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"5714a2781e45c2e0cb06f38fd6810742fc44a210","unresolved":false,"context_lines":[{"line_number":700,"context_line":"                if name:"},{"line_number":701,"context_line":"                    LOG.info(\"Deleting file: %s/%s\", cont, name)"},{"line_number":702,"context_line":"                    client.delete_object(cont, name)"},{"line_number":703,"context_line":"                    # Delete the manifest file"},{"line_number":704,"context_line":"        LOG.info(\"Deleting file: %s/%s\", container, filename)"},{"line_number":705,"context_line":"        client.delete_object(container, filename)"},{"line_number":706,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAS3%2F%2F8S4%3D","line":703,"in_reply_to":"AAAAS3%2F%2F9Bw%3D","updated":"2013-09-17 09:24:37.000000000","message":"Ok.","commit_id":"995c567c6eb00ce250ea8c831ebb976dc801131a"},{"author":{"_account_id":739,"name":"Michael Basnight","email":"mbasnight@gmail.com","username":"hubcap"},"change_message_id":"294d61fbacd7d70cd7a2863d01de4b76bb52547e","unresolved":false,"context_lines":[{"line_number":700,"context_line":"                if name:"},{"line_number":701,"context_line":"                    LOG.info(\"Deleting file: %s/%s\", cont, name)"},{"line_number":702,"context_line":"                    client.delete_object(cont, name)"},{"line_number":703,"context_line":"                    # Delete the manifest file"},{"line_number":704,"context_line":"        LOG.info(\"Deleting file: %s/%s\", container, filename)"},{"line_number":705,"context_line":"        client.delete_object(container, filename)"},{"line_number":706,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAS3%2F%2F8xQ%3D","line":703,"in_reply_to":"AAAAS3%2F%2F9Bw%3D","updated":"2013-09-16 18:08:47.000000000","message":"agreed","commit_id":"995c567c6eb00ce250ea8c831ebb976dc801131a"},{"author":{"_account_id":1925,"name":"Vipul Sabhaya","email":"vipuls@gmail.com","username":"vipuls"},"change_message_id":"38336d5ab0cc877ab1eb4451897a9428381cc7fc","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                            remove_security_group(server,"},{"line_number":197,"context_line":"                                                  security_group[\"name\"])"},{"line_number":198,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":199,"context_line":"                                                      self.context)"},{"line_number":200,"context_line":"            LOG.error(\"Timeout for service changing to active. \""},{"line_number":201,"context_line":"                      \"No usage create-event sent.\")"},{"line_number":202,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F27w%3D","line":199,"updated":"2013-09-18 06:14:25.000000000","message":"I\u0027m not sure this is the best place to put this rollback logic.  At this point, we\u0027re pretty sure that a Server has been created, and any cleanup of associated resources would be handled by a instance delete.","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"c721d5c0c94fdb1f122ad9e779968cd58b9d0a37","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                            remove_security_group(server,"},{"line_number":197,"context_line":"                                                  security_group[\"name\"])"},{"line_number":198,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":199,"context_line":"                                                      self.context)"},{"line_number":200,"context_line":"            LOG.error(\"Timeout for service changing to active. \""},{"line_number":201,"context_line":"                      \"No usage create-event sent.\")"},{"line_number":202,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F2LY%3D","line":199,"in_reply_to":"AAAAS3%2F%2F27w%3D","updated":"2013-09-18 10:05:45.000000000","message":"Than, this code will be moved into forceDelete call","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"53f896ca4f0f07cced9d8656919d40b46df5e34e","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                            remove_security_group(server,"},{"line_number":197,"context_line":"                                                  security_group[\"name\"])"},{"line_number":198,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":199,"context_line":"                                                      self.context)"},{"line_number":200,"context_line":"            LOG.error(\"Timeout for service changing to active. \""},{"line_number":201,"context_line":"                      \"No usage create-event sent.\")"},{"line_number":202,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F2wk%3D","line":199,"in_reply_to":"AAAAS3%2F%2F27w%3D","updated":"2013-09-18 07:18:00.000000000","message":"Until we don\u0027t have force delete API, this should be logic should be placed here. That is why i made it configurable.","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":1925,"name":"Vipul Sabhaya","email":"vipuls@gmail.com","username":"vipuls"},"change_message_id":"38336d5ab0cc877ab1eb4451897a9428381cc7fc","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            self.update_db(compute_instance_id\u003dserver.id, volume_id\u003dvolume_id)"},{"line_number":257,"context_line":"        except Exception as e:"},{"line_number":258,"context_line":"            msg \u003d \"Error creating server and volume for instance.\""},{"line_number":259,"context_line":"            err \u003d inst_models.InstanceTasks.BUILDING_ERROR_SERVER"},{"line_number":260,"context_line":"            self._log_and_raise(e, msg, err)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        device_path \u003d CONF.device_path"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAS3%2F%2F28E%3D","line":259,"updated":"2013-09-18 06:14:25.000000000","message":"I would expect the rollback logic to live here (or any other place that boots a server, and potentially fails).  If we catch an exception here, we will have a Security Group that is dangling.","commit_id":"6da3b3787ad1f0d9fdaaa2318e68a2bbd265b9f3"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":36,"context_line":"from trove.common.utils import poll_until"},{"line_number":37,"context_line":"from trove.instance import models as inst_models"},{"line_number":38,"context_line":"from trove.instance.models import BuiltInstance"},{"line_number":39,"context_line":"from trove.instance.models import DBInstance"},{"line_number":40,"context_line":"from trove.instance.models import FreshInstance"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"from trove.instance.models import InstanceStatus"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Flqc%3D","line":39,"updated":"2013-09-24 09:47:48.000000000","message":"Unused import","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":42,"context_line":"from trove.instance.models import InstanceStatus"},{"line_number":43,"context_line":"from trove.instance.models import InstanceServiceStatus"},{"line_number":44,"context_line":"from trove.instance.models import ServiceStatuses"},{"line_number":45,"context_line":"from trove.instance.tasks import InstanceTasks"},{"line_number":46,"context_line":"from trove.instance.views import get_ip_address"},{"line_number":47,"context_line":"from trove.openstack.common import log as logging"},{"line_number":48,"context_line":"from trove.openstack.common.gettextutils import _"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Flps%3D","line":45,"updated":"2013-09-24 09:47:48.000000000","message":"Unused import","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":51,"context_line":"import trove.common.remote as remote"},{"line_number":52,"context_line":"import trove.backup.models"},{"line_number":53,"context_line":"from trove.extensions.security_group.models import SecurityGroup"},{"line_number":54,"context_line":"from trove.extensions.security_group.models import SecurityGroupRule"},{"line_number":55,"context_line":"from trove.common import exception"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Flo4%3D","line":54,"updated":"2013-09-24 09:47:48.000000000","message":"Unused import","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":52,"context_line":"import trove.backup.models"},{"line_number":53,"context_line":"from trove.extensions.security_group.models import SecurityGroup"},{"line_number":54,"context_line":"from trove.extensions.security_group.models import SecurityGroupRule"},{"line_number":55,"context_line":"from trove.common import exception"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":58,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Flo0%3D","line":55,"updated":"2013-09-24 09:47:48.000000000","message":"Unused import","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"            server \u003d self.nova_client.servers.create("},{"line_number":244,"context_line":"                name, image_id, flavor_id, security_groups\u003dsecurity_groups,"},{"line_number":245,"context_line":"                files\u003dfiles, volume\u003dvolume_ref)"},{"line_number":246,"context_line":"            LOG.debug(_(\"Created new compute instance %s.\") % server.id)"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"            server_dict \u003d server._info"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2Flmg%3D","line":245,"updated":"2013-09-24 09:47:48.000000000","message":"Non-functional change. Please revert.","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7fd1f567a20ac7bb13f759dda96bcec200342c18","unresolved":false,"context_lines":[{"line_number":412,"context_line":"        server \u003d self.nova_client.servers.create("},{"line_number":413,"context_line":"            name, image_id, flavor_id, files\u003dfiles,"},{"line_number":414,"context_line":"            security_groups\u003dsecurity_groups, userdata\u003duserdata,"},{"line_number":415,"context_line":"            block_device_mapping\u003dbdmap)"},{"line_number":416,"context_line":"        LOG.debug(_(\"Created new compute instance %s.\") % server.id)"},{"line_number":417,"context_line":"        return server"},{"line_number":418,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAS3%2F%2FlmU%3D","line":415,"updated":"2013-09-24 09:47:48.000000000","message":"Non-functional change. Please revert.","commit_id":"90b38caca98401531a34df52ec5146c26cad18ca"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"e0cf16711339d71751cac908deecd29e2d135239","unresolved":false,"context_lines":[{"line_number":40,"context_line":"from trove.instance.models import FreshInstance"},{"line_number":41,"context_line":"from trove.instance.models import InstanceStatus"},{"line_number":42,"context_line":"from trove.instance.models import InstanceServiceStatus"},{"line_number":43,"context_line":"from trove.common.instance import ServiceStatuses"},{"line_number":44,"context_line":"from trove.instance.views import get_ip_address"},{"line_number":45,"context_line":"from trove.openstack.common import log as logging"},{"line_number":46,"context_line":"from trove.openstack.common.gettextutils import _"}],"source_content_type":"text/x-python","patch_set":24,"id":"AAAAS3%2F%2FljQ%3D","line":43,"updated":"2013-09-24 10:01:32.000000000","message":"Unused import","commit_id":"93862ac4fd9dcf0cf3830d888d112f4c2eaa6be8"},{"author":{"_account_id":7593,"name":"Haomai Wang","email":"haomaiwang@gmail.com","username":"HaomaiWang"},"change_message_id":"6d005ed0442aaa037e0117d1e5fd7996304ecfae","unresolved":false,"context_lines":[{"line_number":48,"context_line":"from trove.openstack.common import timeutils"},{"line_number":49,"context_line":"import trove.common.remote as remote"},{"line_number":50,"context_line":"import trove.backup.models"},{"line_number":51,"context_line":"from trove.extensions.security_group.models import SecurityGroup"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":54,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FJ2M%3D","line":51,"updated":"2013-10-03 12:17:22.000000000","message":"sort alphabetically","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"24923753f20c4f67710fcb26d85b06e22f0b26e8","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                for security_group in security_groups:"},{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                  self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FOqw%3D","line":197,"updated":"2013-10-01 21:36:57.000000000","message":"Here\u0027s my concern: It\u0027s possible that a guest initializes slower than usual (temporary network degradation), causing a PollTimeout, triggering Security Group deletion, but the instance still can complete init and flip to ACTIVE shortly thereafter. It\u0027s my understanding that at this point the instance will inherit the default Security Group, which could have every port wide open. In my opinion, provisioning should be an atomic all-or-nothing operation, not something involving a partial cleanup that has potential security implications. One could argue that the PollTimeout value could/should be generously large, but that\u0027s only decreasing the probability of the scenario, not avoiding it altogether. I\u0027m fine punting on this until a proper solution is in place for deleting instances stuck in a BUILD state, but I just want to make sure we\u0027re consciously making this decision.","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":7593,"name":"Haomai Wang","email":"haomaiwang@gmail.com","username":"HaomaiWang"},"change_message_id":"6d005ed0442aaa037e0117d1e5fd7996304ecfae","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                for security_group in security_groups:"},{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                  self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FJ0E%3D","line":197,"in_reply_to":"AAAAS3%2F%2FOmE%3D","updated":"2013-10-03 12:17:22.000000000","message":"Yes, there seemed much reviews in rescue \"stuck into BUILDING\", different strategies may worsen conditions.","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":739,"name":"Michael Basnight","email":"mbasnight@gmail.com","username":"hubcap"},"change_message_id":"49e72f5b4e5cc46b37442b3d8d2d14717f3bb975","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                for security_group in security_groups:"},{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                  self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FOmE%3D","line":197,"in_reply_to":"AAAAS3%2F%2FOqw%3D","updated":"2013-10-01 21:53:45.000000000","message":"I think its a fair point. And weve built logic into our delete call such that it deletes this, and dns, even if the instance fails to create. So i dont see the need for saying quotas will be mis-aligned, cuz frankly, the instance quotas will be off by one (cuz the instance is built, but broken), as well as everything else being off by one (dns/secgrp).\n\nAfter talking w/ redthrux about it, i think im ok w/ them being left here, not \"rolled back\" so to speak. I think we need to discuss it at the meeting though so we can have consensus.","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"181c276397fd500b008ddfb00708465e2ab138cc","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FEVk%3D","line":198,"updated":"2013-10-04 23:32:52.000000000","message":"As per the trove meeting this week, we should not be \"rolling back\" any resources. We will clean this up when the instance gets deleted. (If we don\u0027t then that\u0027s a different bug).","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"eda9a3f8b1b747f093b11b60e55c7b039a153bf4","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAATH%2F%2F%2Fbc%3D","line":198,"in_reply_to":"AAAAS3%2F%2FEVk%3D","updated":"2013-10-06 08:35:11.000000000","message":"Accoring to meeting we decided to not delete any of components that is under trove quota management. Security groups aren\u0027t under quote management. So it should be deleted if instance would fail.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"f2b0f3d951f6883c0c93ad586df87758b2e192c5","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAATH%2F%2F%2BdQ%3D","line":198,"in_reply_to":"AAAATH%2F%2F%2Bu0%3D","updated":"2013-10-07 07:00:14.000000000","message":"Just as i suspect. I\u0027ll take a look and fix it.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"a3c234ffdba4d785f0be8ba2565092323a4214d7","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAATH%2F%2F%2Bu0%3D","line":198,"in_reply_to":"AAAATH%2F%2F%2Fbc%3D","updated":"2013-10-07 03:07:34.000000000","message":"When i deleted a failed instance, the delete succeeds but i do see an exception in the API log, SecurityGroupModel not found. So we need to handle the case there the security group does not exist","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"181c276397fd500b008ddfb00708465e2ab138cc","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                if server.addresses !\u003d {}:"},{"line_number":452,"context_line":"                    return True"},{"line_number":453,"context_line":"                elif (server.addresses \u003d\u003d {} and"},{"line_number":454,"context_line":"                        server.status !\u003d InstanceStatus.ERROR):"},{"line_number":455,"context_line":"                    return False"},{"line_number":456,"context_line":"                elif (server.addresses \u003d\u003d {} and"},{"line_number":457,"context_line":"                        server.status \u003d\u003d InstanceStatus.ERROR):"}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FEVw%3D","line":454,"updated":"2013-10-04 23:32:52.000000000","message":"Please remove irrelevant, extra indentation change.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"181c276397fd500b008ddfb00708465e2ab138cc","unresolved":false,"context_lines":[{"line_number":454,"context_line":"                        server.status !\u003d InstanceStatus.ERROR):"},{"line_number":455,"context_line":"                    return False"},{"line_number":456,"context_line":"                elif (server.addresses \u003d\u003d {} and"},{"line_number":457,"context_line":"                        server.status \u003d\u003d InstanceStatus.ERROR):"},{"line_number":458,"context_line":"                    LOG.error(_(\"Instance IP not available, \""},{"line_number":459,"context_line":"                                \"instance (%(instance)s): \""},{"line_number":460,"context_line":"                                \"server had status (%(status)s).\") %"}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FEVs%3D","line":457,"updated":"2013-10-04 23:32:52.000000000","message":"Please remove irrelevant, extra indentation change.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"181c276397fd500b008ddfb00708465e2ab138cc","unresolved":false,"context_lines":[{"line_number":710,"context_line":"                if name:"},{"line_number":711,"context_line":"                    LOG.info(\"Deleting file: %s/%s\", cont, name)"},{"line_number":712,"context_line":"                    client.delete_object(cont, name)"},{"line_number":713,"context_line":"                    # Delete the manifest file"},{"line_number":714,"context_line":"        LOG.info(\"Deleting file: %s/%s\", container, filename)"},{"line_number":715,"context_line":"        client.delete_object(container, filename)"},{"line_number":716,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FEVo%3D","line":713,"updated":"2013-10-04 23:32:52.000000000","message":"This is a comment for the next statement. We should not be indenting it.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"6f67ee5774eb6ba4c5007cf814ed37cc6ea0fdf1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"from trove.instance import models as inst_models"},{"line_number":40,"context_line":"from trove.instance.models import BuiltInstance"},{"line_number":41,"context_line":"from trove.instance.models import FreshInstance"},{"line_number":42,"context_line":"from trove.common.instance import ServiceStatuses"},{"line_number":43,"context_line":"from trove.instance.models import InstanceStatus"},{"line_number":44,"context_line":"from trove.instance.models import InstanceServiceStatus"},{"line_number":45,"context_line":"from trove.instance.views import get_ip_address"}],"source_content_type":"text/x-python","patch_set":35,"id":"AAAATH%2F%2F7Zc%3D","line":42,"updated":"2013-10-07 23:10:35.000000000","message":"Import is not needed.","commit_id":"eed42727c8c3ff727accea1d338fbf79ff339cd6"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"6f67ee5774eb6ba4c5007cf814ed37cc6ea0fdf1","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"AAAATH%2F%2F7Zs%3D","line":198,"updated":"2013-10-07 23:10:35.000000000","message":"In the last trove meeting we decided not to perform any rollback and let the clean up happen on instance delete instead.","commit_id":"eed42727c8c3ff727accea1d338fbf79ff339cd6"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"2376b96d1e297ed25bc5d3326cf45f32eabf0aaa","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                    (self._nova_client.servers."},{"line_number":196,"context_line":"                     remove_security_group(server, security_group[\"name\"]))"},{"line_number":197,"context_line":"                    SecurityGroup.delete_for_instance(self.id,"},{"line_number":198,"context_line":"                                                      self.context)"},{"line_number":199,"context_line":"        except Exception:"},{"line_number":200,"context_line":"            LOG.exception(\"Error during create-event call.\")"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"AAAATH%2F%2F6Zs%3D","line":198,"in_reply_to":"AAAATH%2F%2F7Zs%3D","updated":"2013-10-08 06:03:40.000000000","message":"As you remember we decided not to rollback trove-quota-controlled resources.","commit_id":"eed42727c8c3ff727accea1d338fbf79ff339cd6"}],"trove/tests/unittests/instance/test_models.py":[{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"6f67ee5774eb6ba4c5007cf814ed37cc6ea0fdf1","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        self.assertEqual(None,"},{"line_number":118,"context_line":"                         sec.SecurityGroup."},{"line_number":119,"context_line":"                         delete_for_instance(i_id,"},{"line_number":120,"context_line":"                                             self.context))"}],"source_content_type":"text/x-python","patch_set":35,"id":"AAAATH%2F%2F7UA%3D","line":120,"updated":"2013-10-07 23:10:35.000000000","message":"Again, I\u0027m not sure what we are trying to test with this. If we want to ensure that a previously deleted security group raises an exception if you try to delete it again, we should make sure that we are hitting real trove Security Groups. We should not set up fakes for internal objects, and actually ensure we\u0027re hitting out model code (to test that). \n\nAlso, it looks like you set yourself up to test that an exception is raised, but you are not actually testing that in your assert.","commit_id":"eed42727c8c3ff727accea1d338fbf79ff339cd6"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"2376b96d1e297ed25bc5d3326cf45f32eabf0aaa","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        self.assertEqual(None,"},{"line_number":118,"context_line":"                         sec.SecurityGroup."},{"line_number":119,"context_line":"                         delete_for_instance(i_id,"},{"line_number":120,"context_line":"                                             self.context))"}],"source_content_type":"text/x-python","patch_set":35,"id":"AAAATH%2F%2F6Vs%3D","line":120,"in_reply_to":"AAAATH%2F%2F7UA%3D","updated":"2013-10-08 06:03:40.000000000","message":"Deleting previouse deleted security group would not raise an exception, that is what i\u0027m testing and also testing SecurityGroup.create_for_instance method !","commit_id":"eed42727c8c3ff727accea1d338fbf79ff339cd6"}],"trove/tests/unittests/taskmanager/test_models.py":[{"author":{"_account_id":7593,"name":"Haomai Wang","email":"haomaiwang@gmail.com","username":"HaomaiWang"},"change_message_id":"6d005ed0442aaa037e0117d1e5fd7996304ecfae","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from swiftclient.client import ClientException"},{"line_number":22,"context_line":"from tempfile import NamedTemporaryFile"},{"line_number":23,"context_line":"import uuid"},{"line_number":24,"context_line":"from trove.tests.util import poll_until"},{"line_number":25,"context_line":"import os"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"AAAAS3%2F%2FJ2A%3D","line":24,"updated":"2013-10-03 12:17:22.000000000","message":"separate third-party library and std library","commit_id":"6928896e33020f7edfb0b4de553dcb6e629c31c8"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"181c276397fd500b008ddfb00708465e2ab138cc","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                delete_from_instance(server, sec_gr)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        self.assertNotEqual(server.security_groups, None)"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"class BackupTasksTest(testtools.TestCase):"},{"line_number":215,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":32,"id":"AAAAS3%2F%2FEUg%3D","line":212,"updated":"2013-10-04 23:32:52.000000000","message":"These tests are not testing any of the functionality of the new code we added. Looks like they\u0027re just testing the fake security groups added above, which is of not much value. \n\nNow that we\u0027re planning on not doing the rollback, these tests don\u0027t make much sense either. So they can be possibly removed.","commit_id":"31068c7aa42dc1843f64d3947f0e1a73e61fbb84"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"6f67ee5774eb6ba4c5007cf814ed37cc6ea0fdf1","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from swiftclient.client import ClientException"},{"line_number":22,"context_line":"from tempfile import NamedTemporaryFile"},{"line_number":23,"context_line":"import os"},{"line_number":24,"context_line":"import uuid"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class fake_Server:"}],"source_content_type":"text/x-python","patch_set":35,"id":"AAAATH%2F%2F7Vs%3D","line":24,"updated":"2013-10-07 23:10:35.000000000","message":"Unnecessary import.","commit_id":"eed42727c8c3ff727accea1d338fbf79ff339cd6"}]}