)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add a RequestSpec generation migration script"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In Mitaka, we began to create and persist a RequestSpec object every time"},{"line_number":10,"context_line":"a new instance was requested. Given that instances that were existing before"},{"line_number":11,"context_line":"that commit were not having a related RequestSpec, we needed to check"},{"line_number":12,"context_line":"every time in the conductor methods whether the instance was having this."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_ac2b6d32","line":10,"updated":"2016-05-13 14:58:05.000000000","message":"s/that were existing/that were created/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In Mitaka, we began to create and persist a RequestSpec object every time"},{"line_number":10,"context_line":"a new instance was requested. Given that instances that were existing before"},{"line_number":11,"context_line":"that commit were not having a related RequestSpec, we needed to check"},{"line_number":12,"context_line":"every time in the conductor methods whether the instance was having this."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Now that we are in Newton, we can provide an online data migration script"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_8c2e6940","line":11,"updated":"2016-05-13 14:58:05.000000000","message":"s/were not handing/do not have/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"In Mitaka, we began to create and persist a RequestSpec object every time"},{"line_number":10,"context_line":"a new instance was requested. Given that instances that were existing before"},{"line_number":11,"context_line":"that commit were not having a related RequestSpec, we needed to check"},{"line_number":12,"context_line":"every time in the conductor methods whether the instance was having this."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Now that we are in Newton, we can provide an online data migration script"},{"line_number":15,"context_line":"that is iterating over all the instances (using a marker), verify if the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_ec25755c","line":12,"updated":"2016-05-13 14:58:05.000000000","message":"s/was having this/had a request spec associated with it/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":12,"context_line":"every time in the conductor methods whether the instance was having this."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Now that we are in Newton, we can provide an online data migration script"},{"line_number":15,"context_line":"that is iterating over all the instances (using a marker), verify if the"},{"line_number":16,"context_line":"RequestSpec object is created, and if not, try to persist into that object"},{"line_number":17,"context_line":"the legacy fields we already know."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_cc287136","line":15,"updated":"2016-05-13 14:58:05.000000000","message":"s/that is iterating/that iterates/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":16,"context_line":"RequestSpec object is created, and if not, try to persist into that object"},{"line_number":17,"context_line":"the legacy fields we already know."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The marker is using the request_specs table for persisting itself, and"},{"line_number":20,"context_line":"yes this is an hack but that\u0027s the only solution we agreed for making sure"},{"line_number":21,"context_line":"we were not looping every time on all the instances (which would be a"},{"line_number":22,"context_line":"performance problem). In case we finished to loop over the instances, we"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_2c201d49","line":19,"updated":"2016-05-13 14:58:05.000000000","message":"s/is using the/uses the/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":17,"context_line":"the legacy fields we already know."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The marker is using the request_specs table for persisting itself, and"},{"line_number":20,"context_line":"yes this is an hack but that\u0027s the only solution we agreed for making sure"},{"line_number":21,"context_line":"we were not looping every time on all the instances (which would be a"},{"line_number":22,"context_line":"performance problem). In case we finished to loop over the instances, we"},{"line_number":23,"context_line":"are keeping that marker so that the next call to that migration script"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_0c1b991d","line":20,"updated":"2016-05-13 14:58:05.000000000","message":"s/an hack/a hack, /","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":19,"context_line":"The marker is using the request_specs table for persisting itself, and"},{"line_number":20,"context_line":"yes this is an hack but that\u0027s the only solution we agreed for making sure"},{"line_number":21,"context_line":"we were not looping every time on all the instances (which would be a"},{"line_number":22,"context_line":"performance problem). In case we finished to loop over the instances, we"},{"line_number":23,"context_line":"are keeping that marker so that the next call to that migration script"},{"line_number":24,"context_line":"would not again iterate over all the instances (using the limit)."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_6c1a2519","line":22,"updated":"2016-05-13 14:58:05.000000000","message":"s/In case we finished to loop/When we finish looping/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":20,"context_line":"yes this is an hack but that\u0027s the only solution we agreed for making sure"},{"line_number":21,"context_line":"we were not looping every time on all the instances (which would be a"},{"line_number":22,"context_line":"performance problem). In case we finished to loop over the instances, we"},{"line_number":23,"context_line":"are keeping that marker so that the next call to that migration script"},{"line_number":24,"context_line":"would not again iterate over all the instances (using the limit)."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: I61b9b50436d8bdb8ff06259cc2f876502d688b91"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dab17558_4c15a1e3","line":23,"updated":"2016-05-13 14:58:05.000000000","message":"s/are keeping/keep/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"}],"nova/cmd/manage.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"abb05e08557f49cd44eb4ef75d91a20998ea7121","unresolved":false,"context_lines":[{"line_number":920,"context_line":"    \"\"\"Class for managing the main database.\"\"\""},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"    online_migrations \u003d ("},{"line_number":923,"context_line":"        db.pcidevice_online_data_migration,"},{"line_number":924,"context_line":"        db.computenode_uuids_online_data_migration,"},{"line_number":925,"context_line":"        db.aggregate_uuids_online_data_migration,"},{"line_number":926,"context_line":"        flavor_obj.migrate_flavors,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_2a2fd8a1","line":923,"range":{"start_line":923,"start_character":8,"end_line":923,"end_character":43},"updated":"2016-04-07 14:59:45.000000000","message":"Not related to this change, but we probably want to add a comment saying when each of the migrations can be dropped - for example we should be dropping this one now that we are in Newton.","commit_id":"49d328404cfa9ca717f94c6edf084b20674ad358"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"e930bce5b8887735d830712a02e74dfa5e92efd1","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        db.aggregate_uuids_online_data_migration,"},{"line_number":727,"context_line":"        flavor_obj.migrate_flavors,"},{"line_number":728,"context_line":"        flavor_obj.migrate_flavor_reset_autoincrement,"},{"line_number":729,"context_line":"        request_spec.migrate_instances_without_req_spec,"},{"line_number":730,"context_line":"    )"},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_8633ea12","line":729,"updated":"2016-05-13 14:42:43.000000000","message":"We should start being more descriptive with these. Something like migrate_instances_add_request_spec. Just a nit though.","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"}],"nova/objects/request_spec.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6eda522e18ff9f33a17f59fe61f46b205788bbf1","unresolved":false,"context_lines":[{"line_number":466,"context_line":""},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"# NOTE(sbauza): Yes, this is fine, we just need to have less than 32 chars"},{"line_number":469,"context_line":"FAKE_UUID \u003d \u0027REQSPEC_MIGRATION\u0027"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"@db.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a18252c_f2798583","line":469,"updated":"2016-04-14 14:51:14.000000000","message":"I don\u0027t think it is. Right now our UUID field on the object will take any string. However, there are plans in the works to require proper formatting for this. As such, this will generate a runtime warning for people that have request specs migrated.","commit_id":"6066f42d317d394bfa20e645bd80f63a57f9949e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e897ce1491a54d586d19a685d249d24ece9ae01","unresolved":false,"context_lines":[{"line_number":466,"context_line":""},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"# NOTE(sbauza): Yes, this is fine, we just need to have less than 32 chars"},{"line_number":469,"context_line":"FAKE_UUID \u003d \u0027REQSPEC_MIGRATION\u0027"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"@db.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a18252c_cb593037","line":469,"in_reply_to":"5a18252c_15718b76","updated":"2016-04-14 15:27:27.000000000","message":"Right, I was going to say, using a known UUID, or one of all zeroes would be fine here I think. And we keep running until we\u0027re done, never leaving the marker in once we\u0027re done.","commit_id":"6066f42d317d394bfa20e645bd80f63a57f9949e"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3e349ff04b5eb2a7bbf81b4d71838006c868b419","unresolved":false,"context_lines":[{"line_number":466,"context_line":""},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"# NOTE(sbauza): Yes, this is fine, we just need to have less than 32 chars"},{"line_number":469,"context_line":"FAKE_UUID \u003d \u0027REQSPEC_MIGRATION\u0027"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"@db.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a18252c_bc27dcb4","line":469,"in_reply_to":"5a18252c_cb593037","updated":"2016-04-14 16:10:43.000000000","message":"Yeah, I think a zero UUID could be fine given https://tools.ietf.org/html/rfc4122.html#section-4.4\n\nAt least the bit 7 needs to be 1 for an UUID version 4.","commit_id":"6066f42d317d394bfa20e645bd80f63a57f9949e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"129cf2ad62cab1e8b56d4b74ec448e5c43c871a3","unresolved":false,"context_lines":[{"line_number":466,"context_line":""},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"# NOTE(sbauza): Yes, this is fine, we just need to have less than 32 chars"},{"line_number":469,"context_line":"FAKE_UUID \u003d \u0027REQSPEC_MIGRATION\u0027"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"@db.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a18252c_15718b76","line":469,"in_reply_to":"5a18252c_f2798583","updated":"2016-04-14 15:02:36.000000000","message":"Another option is what I\u0027ve done in https://review.openstack.org/#/c/301916/1/nova/cmd/manage.py which is to indicate the marker using project_id. It may suffer from this same issue, but the UUID very closely matches an actual UUID.\n\nAlternatively using a UUID of all 0s may work here.","commit_id":"6066f42d317d394bfa20e645bd80f63a57f9949e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"af3320af16df6894dd341926c831120e7c3c4848","unresolved":false,"context_lines":[{"line_number":496,"context_line":"    # FIXME(sbauza): The only way to know the latest instance having a"},{"line_number":497,"context_line":"    # RequestSpec persisted is to use a fake marker by using the"},{"line_number":498,"context_line":"    # RequestSpec table to store the instance id. Yeah, that\u0027s very hacky"},{"line_number":499,"context_line":"    # and thus could be modified in case we have a better way."},{"line_number":500,"context_line":"    db_mapping \u003d api_models.RequestSpec()"},{"line_number":501,"context_line":"    db_mapping.update({\u0027instance_uuid\u0027: FAKE_UUID, \u0027spec\u0027: marker})"},{"line_number":502,"context_line":"    db_mapping.save(context.session)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_858984cd","line":499,"updated":"2016-04-15 14:58:29.000000000","message":"Okay, I think we got it.. You think it\u0027s hacky :)\n\nI don\u0027t think this is all that terrible, so I think just one comment at the beginning is probably sufficient :)","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"af3320af16df6894dd341926c831120e7c3c4848","unresolved":false,"context_lines":[{"line_number":508,"context_line":"    if marker:"},{"line_number":509,"context_line":"        # There can be only one persisted marker"},{"line_number":510,"context_line":"        _delete_marker_for_migrate_instances(context, marker)"},{"line_number":511,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":512,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":513,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"},{"line_number":514,"context_line":"                                                    sort_dir\u003d\u0027asc\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_0563b41b","line":511,"updated":"2016-04-15 14:58:29.000000000","message":"If we fail here, we\u0027ve deleted our marker and lost our place, right?","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a8bc557abc15330d97894f4cbc80233361fd42a2","unresolved":false,"context_lines":[{"line_number":508,"context_line":"    if marker:"},{"line_number":509,"context_line":"        # There can be only one persisted marker"},{"line_number":510,"context_line":"        _delete_marker_for_migrate_instances(context, marker)"},{"line_number":511,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":512,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":513,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"},{"line_number":514,"context_line":"                                                    sort_dir\u003d\u0027asc\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_3c04dabe","line":511,"in_reply_to":"1a122d0e_0563b41b","updated":"2016-04-18 08:13:55.000000000","message":"Good point, I should remove the marker a bit later.","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"af3320af16df6894dd341926c831120e7c3c4848","unresolved":false,"context_lines":[{"line_number":536,"context_line":"                instance.pci_requests,"},{"line_number":537,"context_line":"                filter_properties, None, instance.availability_zone"},{"line_number":538,"context_line":"            )"},{"line_number":539,"context_line":"            request_spec.create()"},{"line_number":540,"context_line":"            count_hit +\u003d 1"},{"line_number":541,"context_line":"    if count_all \u003e 0 and count_all \u003d\u003d max_count:"},{"line_number":542,"context_line":"        # We need to persist the last instance ID that we checked as the next"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_e535d8f7","line":539,"updated":"2016-04-15 14:58:29.000000000","message":"Could you break this all into a helper function? Putting it all in the except: clause is kinda messy I think.","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a8bc557abc15330d97894f4cbc80233361fd42a2","unresolved":false,"context_lines":[{"line_number":536,"context_line":"                instance.pci_requests,"},{"line_number":537,"context_line":"                filter_properties, None, instance.availability_zone"},{"line_number":538,"context_line":"            )"},{"line_number":539,"context_line":"            request_spec.create()"},{"line_number":540,"context_line":"            count_hit +\u003d 1"},{"line_number":541,"context_line":"    if count_all \u003e 0 and count_all \u003d\u003d max_count:"},{"line_number":542,"context_line":"        # We need to persist the last instance ID that we checked as the next"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_7c2c924a","line":539,"in_reply_to":"1a122d0e_e535d8f7","updated":"2016-04-18 08:13:55.000000000","message":"Agreed.","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"af3320af16df6894dd341926c831120e7c3c4848","unresolved":false,"context_lines":[{"line_number":548,"context_line":"            limit\u003d1,"},{"line_number":549,"context_line":"            marker\u003dinstances[-1].uuid)"},{"line_number":550,"context_line":"        if nexts:"},{"line_number":551,"context_line":"            _set_marker_for_migrate_instances(context, instance.uuid)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"    return count_all, count_hit"},{"line_number":554,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_a503e00e","line":551,"updated":"2016-04-15 14:58:29.000000000","message":"I think maybe your set_marker method should do the delete and set operation. Otherwise these are separate transactions and not atomic, right? I think we have a race-with-death issue here where we might drop the marker out if we fail at the right spot, no?","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a8bc557abc15330d97894f4cbc80233361fd42a2","unresolved":false,"context_lines":[{"line_number":548,"context_line":"            limit\u003d1,"},{"line_number":549,"context_line":"            marker\u003dinstances[-1].uuid)"},{"line_number":550,"context_line":"        if nexts:"},{"line_number":551,"context_line":"            _set_marker_for_migrate_instances(context, instance.uuid)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"    return count_all, count_hit"},{"line_number":554,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_bcb0eade","line":551,"in_reply_to":"1a122d0e_a503e00e","updated":"2016-04-18 08:13:55.000000000","message":"Deleting the previous marker when setting the new one is okay, but I feel we should still delete the marker if we are at the end of the loops with all the instances having been checked.\n\nIf not, it would mean that trying again to verify all the instances wouldn\u0027t be possible (so the migration script would not be idempotent, right?)\n\nTo be clear, I\u0027m not set yet, just wondering whether we should accept to verify again the list of instances from the beginning or we should just say \"eh, you did that previously, fair enough\".\n\nThoughts ?","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d1550b8885c6469decf5cd3c13d78ec30d20c0ea","unresolved":false,"context_lines":[{"line_number":548,"context_line":"            limit\u003d1,"},{"line_number":549,"context_line":"            marker\u003dinstances[-1].uuid)"},{"line_number":550,"context_line":"        if nexts:"},{"line_number":551,"context_line":"            _set_marker_for_migrate_instances(context, instance.uuid)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"    return count_all, count_hit"},{"line_number":554,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_b03abc96","line":551,"in_reply_to":"1a122d0e_bcb0eade","updated":"2016-04-18 13:36:28.000000000","message":"Yes, definitely, but make that the special case. So I think, at the end of this:\n\n if done:\n     delete_marker()\n else:\n     set_new_marker_and_delete_old()\n\nyes?","commit_id":"4d811bf6813a79055e099cfa7637323620305457"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa15a463e8c0f515a8b6ac6b316cd6722ff32cf5","unresolved":false,"context_lines":[{"line_number":520,"context_line":""},{"line_number":521,"context_line":"def migrate_instances_without_req_spec(context, max_count):"},{"line_number":522,"context_line":"    \"\"\"Creates and persists a RequestSpec per instance not yet having it.\"\"\""},{"line_number":523,"context_line":"    marker \u003d _get_marker_for_migrate_instances(context)"},{"line_number":524,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":525,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":526,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_931eaa61","line":523,"updated":"2016-04-18 13:51:20.000000000","message":"If we have already migrated everything and then this gets run again (which is can) then will we run through all the instances again? I think we need a way to short-circuit here if there is no marker, but we\u0027re already done, right?","commit_id":"643232dd21e7e91e18f622ad79ef7528c4998a72"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a50b1854ec8a051545961b9684c24d6e92da9f59","unresolved":false,"context_lines":[{"line_number":520,"context_line":""},{"line_number":521,"context_line":"def migrate_instances_without_req_spec(context, max_count):"},{"line_number":522,"context_line":"    \"\"\"Creates and persists a RequestSpec per instance not yet having it.\"\"\""},{"line_number":523,"context_line":"    marker \u003d _get_marker_for_migrate_instances(context)"},{"line_number":524,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":525,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":526,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_4099b61e","line":523,"in_reply_to":"1a122d0e_931eaa61","updated":"2016-04-19 08:37:05.000000000","message":"That\u0027s the open question I had in PS3. I was feeling we should run again against all the instances once we\u0027re done, but I could then leave the marker with the last instance in case we don\u0027t want to run again...","commit_id":"643232dd21e7e91e18f622ad79ef7528c4998a72"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3d831c75ae7da16d70e405bbcad11ff8dfa9d75c","unresolved":false,"context_lines":[{"line_number":479,"context_line":""},{"line_number":480,"context_line":"@db.api_context_manager.reader"},{"line_number":481,"context_line":"def _get_marker_for_migrate_instances(context):"},{"line_number":482,"context_line":"    req_spec \u003d (context.session.query(api_models.RequestSpec).filter_by("},{"line_number":483,"context_line":"                instance_uuid\u003dFAKE_UUID)).first()"},{"line_number":484,"context_line":"    marker \u003d req_spec[\u0027spec\u0027] if req_spec else None"},{"line_number":485,"context_line":"    return marker"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_5913d20d","line":484,"range":{"start_line":482,"start_character":4,"end_line":484,"end_character":51},"updated":"2016-05-12 19:31:50.000000000","message":"On the first pass this returns None for marker right because there is no request spec for this fake uuid.","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e20cc3b2039dc6f4ec61ceb9ec64d2d683c5f2e6","unresolved":false,"context_lines":[{"line_number":479,"context_line":""},{"line_number":480,"context_line":"@db.api_context_manager.reader"},{"line_number":481,"context_line":"def _get_marker_for_migrate_instances(context):"},{"line_number":482,"context_line":"    req_spec \u003d (context.session.query(api_models.RequestSpec).filter_by("},{"line_number":483,"context_line":"                instance_uuid\u003dFAKE_UUID)).first()"},{"line_number":484,"context_line":"    marker \u003d req_spec[\u0027spec\u0027] if req_spec else None"},{"line_number":485,"context_line":"    return marker"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_5902b270","line":484,"range":{"start_line":482,"start_character":4,"end_line":484,"end_character":51},"in_reply_to":"dab17558_5913d20d","updated":"2016-05-12 19:39:54.000000000","message":"Yup, that\u0027s the intended behaviour, which then calls the .get_by_filters with marker\u003dNone which is fine","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3d831c75ae7da16d70e405bbcad11ff8dfa9d75c","unresolved":false,"context_lines":[{"line_number":489,"context_line":"def _set_or_delete_marker_for_migrate_instances(context, marker\u003dNone):"},{"line_number":490,"context_line":"    # We need to delete the old marker anyway, which no longer corresponds to"},{"line_number":491,"context_line":"    # the last instance we checked (if there was a marker)..."},{"line_number":492,"context_line":"    context.session.query(api_models.RequestSpec).filter_by("},{"line_number":493,"context_line":"        instance_uuid\u003dFAKE_UUID).delete()"},{"line_number":494,"context_line":"    if marker is not None:"},{"line_number":495,"context_line":"        # ... but there can be a new marker to set"},{"line_number":496,"context_line":"        db_mapping \u003d api_models.RequestSpec()"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_7925f65a","line":493,"range":{"start_line":492,"start_character":4,"end_line":493,"end_character":41},"updated":"2016-05-12 19:31:50.000000000","message":"How does this work on the first pass if we never actually stored a request spec with the fake uuid?","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e20cc3b2039dc6f4ec61ceb9ec64d2d683c5f2e6","unresolved":false,"context_lines":[{"line_number":489,"context_line":"def _set_or_delete_marker_for_migrate_instances(context, marker\u003dNone):"},{"line_number":490,"context_line":"    # We need to delete the old marker anyway, which no longer corresponds to"},{"line_number":491,"context_line":"    # the last instance we checked (if there was a marker)..."},{"line_number":492,"context_line":"    context.session.query(api_models.RequestSpec).filter_by("},{"line_number":493,"context_line":"        instance_uuid\u003dFAKE_UUID).delete()"},{"line_number":494,"context_line":"    if marker is not None:"},{"line_number":495,"context_line":"        # ... but there can be a new marker to set"},{"line_number":496,"context_line":"        db_mapping \u003d api_models.RequestSpec()"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_197dcae7","line":493,"range":{"start_line":492,"start_character":4,"end_line":493,"end_character":41},"in_reply_to":"dab17558_7925f65a","updated":"2016-05-12 19:39:54.000000000","message":"So, I verified that a session.filter_by_().delete() is fine with SQLA if nothing is still found (not raising something like an AttributeError for a NoneType...","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"971260dd0d8bcd77033da5a189bf812083bf383d","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"def _create_minimal_request_spec(context, instance):"},{"line_number":502,"context_line":"    image \u003d utils.get_image_from_system_metadata("},{"line_number":503,"context_line":"        instance.system_metadata)"},{"line_number":504,"context_line":"    # TODO(sbauza): Modify that once setup_instance_group() accepts a"},{"line_number":505,"context_line":"    # RequestSpec object"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_79c976ac","line":502,"range":{"start_line":502,"start_character":12,"end_line":502,"end_character":48},"updated":"2016-05-12 19:27:51.000000000","message":"Use instance.image_meta","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"da5e46b4810d550ef6234f6bf735dcf95eff98dd","unresolved":false,"context_lines":[{"line_number":500,"context_line":""},{"line_number":501,"context_line":"def _create_minimal_request_spec(context, instance):"},{"line_number":502,"context_line":"    image \u003d utils.get_image_from_system_metadata("},{"line_number":503,"context_line":"        instance.system_metadata)"},{"line_number":504,"context_line":"    # TODO(sbauza): Modify that once setup_instance_group() accepts a"},{"line_number":505,"context_line":"    # RequestSpec object"},{"line_number":506,"context_line":"    request_spec \u003d {\u0027instance_properties\u0027: {\u0027uuid\u0027: instance.uuid}}"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_d6731114","line":503,"range":{"start_line":503,"start_character":8,"end_line":503,"end_character":32},"updated":"2016-05-12 19:13:29.000000000","message":"When you got these from the database via get_by_filters, you didn\u0027t specify expected_attrs\u003d[\u0027system_metadata\u0027] so we\u0027re lazy-loading this for each instance each time we loop, right? That\u0027s no bueno.","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3d831c75ae7da16d70e405bbcad11ff8dfa9d75c","unresolved":false,"context_lines":[{"line_number":511,"context_line":"    # object using the existing we know and previously saved."},{"line_number":512,"context_line":"    request_spec \u003d objects.RequestSpec.from_components("},{"line_number":513,"context_line":"        context, instance.uuid, image,"},{"line_number":514,"context_line":"        instance.flavor, instance.numa_topology,"},{"line_number":515,"context_line":"        instance.pci_requests,"},{"line_number":516,"context_line":"        filter_properties, None, instance.availability_zone"},{"line_number":517,"context_line":"    )"},{"line_number":518,"context_line":"    request_spec.create()"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_3936ceb4","line":515,"range":{"start_line":514,"start_character":8,"end_line":515,"end_character":29},"updated":"2016-05-12 19:31:50.000000000","message":"You\u0027re also going to be lazy-loading these aren\u0027t you?","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a84028be033f26613cbe9544a880ff08a8e80b6b","unresolved":false,"context_lines":[{"line_number":522,"context_line":"    \"\"\"Creates and persists a RequestSpec per instance not yet having it.\"\"\""},{"line_number":523,"context_line":"    marker \u003d _get_marker_for_migrate_instances(context)"},{"line_number":524,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":525,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":526,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"},{"line_number":527,"context_line":"                                                    sort_dir\u003d\u0027asc\u0027,"},{"line_number":528,"context_line":"                                                    limit\u003dmax_count,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_d6d791eb","line":525,"range":{"start_line":525,"start_character":72,"end_line":525,"end_character":77},"updated":"2016-05-12 19:11:09.000000000","message":"The deleted field is an id, not a boolean, so shouldn\u0027t this be 0?","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"23f3966b7dbfde5d4e0254b0245132288dd0483f","unresolved":false,"context_lines":[{"line_number":522,"context_line":"    \"\"\"Creates and persists a RequestSpec per instance not yet having it.\"\"\""},{"line_number":523,"context_line":"    marker \u003d _get_marker_for_migrate_instances(context)"},{"line_number":524,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":525,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":526,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"},{"line_number":527,"context_line":"                                                    sort_dir\u003d\u0027asc\u0027,"},{"line_number":528,"context_line":"                                                    limit\u003dmax_count,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_d6e1519c","line":525,"range":{"start_line":525,"start_character":72,"end_line":525,"end_character":77},"in_reply_to":"dab17558_d61cb1c3","updated":"2016-05-12 19:16:26.000000000","message":"Doesn\u0027t look like your functional test has a deleted instance in it either.","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"16c21344d0f9ff79d994a6feedd8976435f5ed20","unresolved":false,"context_lines":[{"line_number":522,"context_line":"    \"\"\"Creates and persists a RequestSpec per instance not yet having it.\"\"\""},{"line_number":523,"context_line":"    marker \u003d _get_marker_for_migrate_instances(context)"},{"line_number":524,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":525,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":526,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"},{"line_number":527,"context_line":"                                                    sort_dir\u003d\u0027asc\u0027,"},{"line_number":528,"context_line":"                                                    limit\u003dmax_count,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_d61cb1c3","line":525,"range":{"start_line":525,"start_character":72,"end_line":525,"end_character":77},"in_reply_to":"dab17558_d6d791eb","updated":"2016-05-12 19:12:57.000000000","message":"You\u0027re totally right, I\u0027m a dumb ass.","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"203e0b815ca199bae8b7d8c8ce2c6b2383e4cd59","unresolved":false,"context_lines":[{"line_number":522,"context_line":"    \"\"\"Creates and persists a RequestSpec per instance not yet having it.\"\"\""},{"line_number":523,"context_line":"    marker \u003d _get_marker_for_migrate_instances(context)"},{"line_number":524,"context_line":"    instances \u003d objects.InstanceList.get_by_filters(context,"},{"line_number":525,"context_line":"                                                    filters\u003d{\u0027deleted\u0027: False},"},{"line_number":526,"context_line":"                                                    sort_key\u003d\u0027created_at\u0027,"},{"line_number":527,"context_line":"                                                    sort_dir\u003d\u0027asc\u0027,"},{"line_number":528,"context_line":"                                                    limit\u003dmax_count,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_cf7d30b3","line":525,"range":{"start_line":525,"start_character":72,"end_line":525,"end_character":77},"in_reply_to":"dab17558_d6e1519c","updated":"2016-05-12 20:28:10.000000000","message":"Hum, actually, I remember why I wrote that, it\u0027s because https://github.com/openstack/nova/blob/92aaa4d40bcd291e9d354e16c9986a057ee42c73/nova/db/sqlalchemy/api.py#L2312-L2313\n\nWhich means we need a boolean here (or remove it here and just provide the functional check)","commit_id":"bf3c03984f1ef6cd937849e32d6fc1a790200384"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":465,"context_line":""},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"# NOTE(sbauza): Since verifying a huge list of instances can be a performance"},{"line_number":468,"context_line":"# impact, we need to use a marker for only checking a set of them rather."},{"line_number":469,"context_line":"# As the current model doesn\u0027t expose a way to persist that marker, we propose"},{"line_number":470,"context_line":"# here to use the request_specs table with a fake (and impossible) instance"},{"line_number":471,"context_line":"# UUID where the related spec field (which is Text) would be the marker, ie."}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_6c0345b9","line":468,"updated":"2016-05-13 14:58:05.000000000","message":"s/ rather//","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":508,"context_line":"    scheduler_utils.setup_instance_group(context, request_spec,"},{"line_number":509,"context_line":"                                         filter_properties)"},{"line_number":510,"context_line":"    # This is an old instance. Let\u0027s try to populate a RequestSpec"},{"line_number":511,"context_line":"    # object using the existing we know and previously saved."},{"line_number":512,"context_line":"    request_spec \u003d objects.RequestSpec.from_components("},{"line_number":513,"context_line":"        context, instance.uuid, image,"},{"line_number":514,"context_line":"        instance.flavor, instance.numa_topology,"}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_4cfe41af","line":511,"updated":"2016-05-13 14:58:05.000000000","message":"s/existing we know and previously saved/existing information we have previously saved/","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"}],"nova/tests/functional/db/test_request_spec.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa15a463e8c0f515a8b6ac6b316cd6722ff32cf5","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        instance_uuid\u003dinstance_uuid).delete()"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"class RequestSpecInstanceMigrationTestCase("},{"line_number":84,"context_line":"        integrated_helpers._IntegratedTestBase):"},{"line_number":85,"context_line":"    api_major_version \u003d \u0027v2.1\u0027"},{"line_number":86,"context_line":"    _image_ref_parameter \u003d \u0027imageRef\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_53c5328c","line":83,"updated":"2016-04-18 13:51:20.000000000","message":"I think you should also have a test where you create several instances through the api and then make sure that nothing happens when you run the migration. In other words, make sure that new instances are created without any residue to clean up.","commit_id":"643232dd21e7e91e18f622ad79ef7528c4998a72"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa15a463e8c0f515a8b6ac6b316cd6722ff32cf5","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        self.context \u003d context.get_admin_context()"},{"line_number":93,"context_line":"        fake_network.set_stub_network_methods(self)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def test_migration(self):"},{"line_number":96,"context_line":"        request \u003d self._build_minimal_create_server_request()"},{"line_number":97,"context_line":"        # Create 5 instances that would set a RequestSpec object"},{"line_number":98,"context_line":"        request.update({\u0027max_count\u0027: 5})"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_53ded26d","line":95,"updated":"2016-04-18 13:51:20.000000000","message":"I\u0027m not sure how this is different than the test below.","commit_id":"643232dd21e7e91e18f622ad79ef7528c4998a72"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a50b1854ec8a051545961b9684c24d6e92da9f59","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        self.context \u003d context.get_admin_context()"},{"line_number":93,"context_line":"        fake_network.set_stub_network_methods(self)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def test_migration(self):"},{"line_number":96,"context_line":"        request \u003d self._build_minimal_create_server_request()"},{"line_number":97,"context_line":"        # Create 5 instances that would set a RequestSpec object"},{"line_number":98,"context_line":"        request.update({\u0027max_count\u0027: 5})"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_fae34a31","line":95,"in_reply_to":"1a122d0e_53ded26d","updated":"2016-04-19 08:37:05.000000000","message":"Just not using the marker. But I\u0027ll factorize the tests.","commit_id":"643232dd21e7e91e18f622ad79ef7528c4998a72"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"aa15a463e8c0f515a8b6ac6b316cd6722ff32cf5","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # We removed the marker in the previous call because we iterated over"},{"line_number":151,"context_line":"        # all the instances, now we start to look again at all the instances"},{"line_number":152,"context_line":"        # so as to be idempotent."},{"line_number":153,"context_line":"        self.assertEqual(5, match)"},{"line_number":154,"context_line":"        self.assertEqual(0, done)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        # Make sure all instances have now a related RequestSpec"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_13a61a10","line":153,"updated":"2016-04-18 13:51:20.000000000","message":"This is not right. Once all the instances are migrated, this must match zero so that we know to move on to the next online migration script. Zero match and zero done means \"everything is migrated\".","commit_id":"643232dd21e7e91e18f622ad79ef7528c4998a72"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a50b1854ec8a051545961b9684c24d6e92da9f59","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # We removed the marker in the previous call because we iterated over"},{"line_number":151,"context_line":"        # all the instances, now we start to look again at all the instances"},{"line_number":152,"context_line":"        # so as to be idempotent."},{"line_number":153,"context_line":"        self.assertEqual(5, match)"},{"line_number":154,"context_line":"        self.assertEqual(0, done)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        # Make sure all instances have now a related RequestSpec"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a122d0e_bac0b27d","line":153,"in_reply_to":"1a122d0e_13a61a10","updated":"2016-04-19 08:37:05.000000000","message":"okay, fair point, it was my open question as I said earlier.","commit_id":"643232dd21e7e91e18f622ad79ef7528c4998a72"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bf917681ada31dbfbd6079799bd1db553677091","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        for i in range(0, old):"},{"line_number":108,"context_line":"            _delete_request_spec(self.context, self.instances[i].uuid)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        # Just add a deleted instance to make sure we don\u0027t create"},{"line_number":111,"context_line":"        # a RequestSpec object for it."},{"line_number":112,"context_line":"        deleted_instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":113,"context_line":"        del deleted_instance.id"},{"line_number":114,"context_line":"        deleted_instance.create()"},{"line_number":115,"context_line":"        deleted_instance.destroy()"},{"line_number":116,"context_line":"        # Make sure we have the deleted instance only soft-deleted in DB"},{"line_number":117,"context_line":"        deleted_instances \u003d objects.InstanceList.get_by_filters("},{"line_number":118,"context_line":"            self.context, filters\u003d{\u0027deleted\u0027: True})"},{"line_number":119,"context_line":"        self.assertEqual(1, len(deleted_instances))"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def test_migration(self):"},{"line_number":122,"context_line":"        self._create_instances(old\u003d2, total\u003d5)"}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_2ca3fd9f","line":119,"range":{"start_line":110,"start_character":8,"end_line":119,"end_character":51},"updated":"2016-05-13 15:00:41.000000000","message":"This seems a bit odd to do in a functional test, especially when the other instances are created through the REST API. Why not just create an extra instance via REST API and then delete that (also via REST API)?","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"df9d757e020a48a9f286913e2104688f489bc20d","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        match, done \u003d request_spec.migrate_instances_without_req_spec("},{"line_number":154,"context_line":"            self.context, 50)"},{"line_number":155,"context_line":"        self.assertEqual(5, match)"},{"line_number":156,"context_line":"        self.assertEqual(0, done)"}],"source_content_type":"text/x-python","patch_set":10,"id":"dab17558_acd24d32","line":156,"updated":"2016-05-13 14:58:05.000000000","message":"Very nice functional tests, Sylvain. ++","commit_id":"fea1a3ecd3a6e68d28b49d09fd97cf229281e48e"}]}