)]}'
{"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c7680a2978f3f1baff4d10605817be5ca2293e81","unresolved":false,"context_lines":[{"line_number":1810,"context_line":"    ec2_instance_create(context, instance_ref[\u0027uuid\u0027])"},{"line_number":1811,"context_line":""},{"line_number":1812,"context_line":"    if deleted:"},{"line_number":1813,"context_line":"        instance_ref.soft_delete(context.session)"},{"line_number":1814,"context_line":""},{"line_number":1815,"context_line":"    return instance_ref"},{"line_number":1816,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_8231a05c","line":1813,"updated":"2016-11-28 16:18:01.000000000","message":"I\u0027m a little bit worried about this. It will set the deleted and deleted_at fields. The instance won\u0027t really have been soft-deleted though, at least not in the sense of how it works via the API when CONF.reclaim_instance_interval is set and we change the vm_state to \"soft-delete\".\n\nThe _reclaim_queued_deletes periodic task in the compute manager won\u0027t hard delete this instance because the vm_state isn\u0027t set to \u0027soft-delete\u0027. Hell on instance create the vm_state at this point might be \u0027building\u0027 which would be weird.\n\nI tend to think we should just convert False to 0 in _handle_objects_related_type_conversions and just error out if trying to create an instance with deleted\u003dTrue, as it\u0027s something we don\u0027t support today, and we should probably think about if/when we want to support that case.\n\nI might be overthinking this too, it just doesn\u0027t sit right with me.","commit_id":"a752f8c7fe6f722255133a95b283e72d8de84733"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f037015daf36be720e10e1136346ee10c110f36c","unresolved":false,"context_lines":[{"line_number":1810,"context_line":"    ec2_instance_create(context, instance_ref[\u0027uuid\u0027])"},{"line_number":1811,"context_line":""},{"line_number":1812,"context_line":"    if deleted:"},{"line_number":1813,"context_line":"        instance_ref.soft_delete(context.session)"},{"line_number":1814,"context_line":""},{"line_number":1815,"context_line":"    return instance_ref"},{"line_number":1816,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_c5e71daa","line":1813,"in_reply_to":"5a74a57a_058035f4","updated":"2016-11-28 18:18:29.000000000","message":"I know, I\u0027m just thinking about the impacts of doing something like this. For example, listing servers by default wouldn\u0027t show this server, but would if you did \u0027nova list --deleted\u0027 at which point we have a server listed as deleted but it\u0027s status might be BUILD, which is weird.","commit_id":"a752f8c7fe6f722255133a95b283e72d8de84733"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b64e237d2545fbf1d4e00ff8a3375a1afbb29625","unresolved":false,"context_lines":[{"line_number":1810,"context_line":"    ec2_instance_create(context, instance_ref[\u0027uuid\u0027])"},{"line_number":1811,"context_line":""},{"line_number":1812,"context_line":"    if deleted:"},{"line_number":1813,"context_line":"        instance_ref.soft_delete(context.session)"},{"line_number":1814,"context_line":""},{"line_number":1815,"context_line":"    return instance_ref"},{"line_number":1816,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_058035f4","line":1813,"in_reply_to":"5a74a57a_8231a05c","updated":"2016-11-28 18:11:14.000000000","message":"This is not the same as the high-level soft delete, this is the deleted\u003d$id soft-delete pattern from oslo.db. Totally different from the user doing a soft-delete from the client.","commit_id":"a752f8c7fe6f722255133a95b283e72d8de84733"}],"nova/tests/unit/db/test_db_api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2f7b17d1573effb7289890272b0b30651fa5b4e","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def create_deleted(self):"},{"line_number":197,"context_line":"        values \u003d {\u0027user_id\u0027: \u0027fake\u0027,"},{"line_number":198,"context_line":"                  \u0027project_id\u0027: \u0027fake\u0027,"},{"line_number":199,"context_line":"                  \u0027deleted\u0027: 1}"},{"line_number":200,"context_line":"        inst \u003d db.instance_create(self.context, values)"},{"line_number":201,"context_line":"        self.assertNotEqual(0, inst.deleted)"},{"line_number":202,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_8250c033","line":199,"updated":"2016-11-28 16:21:15.000000000","message":"nit: wouldn\u0027t we actually be passing deleted\u003dTrue if using an instance object because the deleted field on the instance object is a boolean:\n\nhttps://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/fields.py#L438","commit_id":"a752f8c7fe6f722255133a95b283e72d8de84733"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b64e237d2545fbf1d4e00ff8a3375a1afbb29625","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def create_deleted(self):"},{"line_number":197,"context_line":"        values \u003d {\u0027user_id\u0027: \u0027fake\u0027,"},{"line_number":198,"context_line":"                  \u0027project_id\u0027: \u0027fake\u0027,"},{"line_number":199,"context_line":"                  \u0027deleted\u0027: 1}"},{"line_number":200,"context_line":"        inst \u003d db.instance_create(self.context, values)"},{"line_number":201,"context_line":"        self.assertNotEqual(0, inst.deleted)"},{"line_number":202,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_65bf29aa","line":199,"in_reply_to":"5a74a57a_8250c033","updated":"2016-11-28 18:11:14.000000000","message":"Yes, but neither 1 nor True is correct, so we just need something truthy. Since this is a db-api-level test, I was going for matching the type. Maybe I should follow up with a correction in create() to convert True-\u003e1 and add a test to make sure it works from that level?","commit_id":"a752f8c7fe6f722255133a95b283e72d8de84733"}]}
