)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7fca603ffb416b8f6b77af9390ba1d77d4253956","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Previously, in the Instance.fill_faults() method, we were getting all"},{"line_number":14,"context_line":"instance fault records for a set of instances having one of a set of"},{"line_number":15,"context_line":"supplied instance UUIDs and then iterating over those faults and"},{"line_number":16,"context_line":"returning a dict of instance UUID to the first fault returned (which"},{"line_number":17,"context_line":"happened to be the latest fault because of ordering the SQL query by"},{"line_number":18,"context_line":"created_at)."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This patch adds a new InstanceFaultList.get_latest_by_instance_uuids()"},{"line_number":21,"context_line":"method that does some SQL-fu to only return the latest fault records for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5201f7_c187c510","line":18,"range":{"start_line":16,"start_character":62,"end_line":18,"end_character":12},"updated":"2016-12-29 16:41:39.000000000","message":"Wasn\u0027t it really the oldest fault because the sql query was ordering by created_at in descending order?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e591451dd3e275ff1b8168c8f95108b700c3d0b2","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Previously, in the Instance.fill_faults() method, we were getting all"},{"line_number":14,"context_line":"instance fault records for a set of instances having one of a set of"},{"line_number":15,"context_line":"supplied instance UUIDs and then iterating over those faults and"},{"line_number":16,"context_line":"returning a dict of instance UUID to the first fault returned (which"},{"line_number":17,"context_line":"happened to be the latest fault because of ordering the SQL query by"},{"line_number":18,"context_line":"created_at)."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This patch adds a new InstanceFaultList.get_latest_by_instance_uuids()"},{"line_number":21,"context_line":"method that does some SQL-fu to only return the latest fault records for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5201f7_7c623eda","line":18,"range":{"start_line":16,"start_character":62,"end_line":18,"end_character":12},"in_reply_to":"ba5201f7_2187b110","updated":"2016-12-29 17:33:39.000000000","message":"Ignore this comment, I\u0027ve seen the error of my ways.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d2e7ada161964f452cf54156b3b50f862ce06b0b","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Previously, in the Instance.fill_faults() method, we were getting all"},{"line_number":14,"context_line":"instance fault records for a set of instances having one of a set of"},{"line_number":15,"context_line":"supplied instance UUIDs and then iterating over those faults and"},{"line_number":16,"context_line":"returning a dict of instance UUID to the first fault returned (which"},{"line_number":17,"context_line":"happened to be the latest fault because of ordering the SQL query by"},{"line_number":18,"context_line":"created_at)."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This patch adds a new InstanceFaultList.get_latest_by_instance_uuids()"},{"line_number":21,"context_line":"method that does some SQL-fu to only return the latest fault records for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5201f7_2187b110","line":18,"range":{"start_line":16,"start_character":62,"end_line":18,"end_character":12},"in_reply_to":"ba5201f7_c187c510","updated":"2016-12-29 16:50:34.000000000","message":"well.. yeah. :)","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"a7348f04b3b427d897e0ff3c7dd79229ee4a3a73","unresolved":false,"context_lines":[{"line_number":6028,"context_line":"        #    SELECT instance_uuid, MAX(id) AS max_id"},{"line_number":6029,"context_line":"        #    FROM instance_faults"},{"line_number":6030,"context_line":"        #    WHERE instance_uuid IN ( ... )"},{"line_number":6031,"context_line":"        #    AND deleted IS NULL"},{"line_number":6032,"context_line":"        #    GROUP BY instance_uuid"},{"line_number":6033,"context_line":"        #  ) AS latest_faults"},{"line_number":6034,"context_line":"        #    ON instance_faults.id \u003d latest_faults.max_id;"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6eadb0_a05f1c71","line":6031,"range":{"start_line":6031,"start_character":25,"end_line":6031,"end_character":32},"updated":"2016-12-13 08:27:38.000000000","message":"nit, there should be \"deleted \u003d 0\"","commit_id":"9be3ea227d3249042f849b7f92cab7f2c7f6b14e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"b4b881fb8ccd1db837a0a6dfea75c94397334164","unresolved":false,"context_lines":[{"line_number":6028,"context_line":"        #    SELECT instance_uuid, MAX(id) AS max_id"},{"line_number":6029,"context_line":"        #    FROM instance_faults"},{"line_number":6030,"context_line":"        #    WHERE instance_uuid IN ( ... )"},{"line_number":6031,"context_line":"        #    AND deleted IS NULL"},{"line_number":6032,"context_line":"        #    GROUP BY instance_uuid"},{"line_number":6033,"context_line":"        #  ) AS latest_faults"},{"line_number":6034,"context_line":"        #    ON instance_faults.id \u003d latest_faults.max_id;"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6eadb0_58470bbb","line":6031,"range":{"start_line":6031,"start_character":25,"end_line":6031,"end_character":32},"in_reply_to":"1a6eadb0_a05f1c71","updated":"2016-12-13 14:12:31.000000000","message":"Done","commit_id":"9be3ea227d3249042f849b7f92cab7f2c7f6b14e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"4ddd5b861bbdb65e4e7ffe6d37b32b49af3f81be","unresolved":false,"context_lines":[{"line_number":6034,"context_line":"        #    ON instance_faults.id \u003d latest_faults.max_id;"},{"line_number":6035,"context_line":"        faults_tbl \u003d models.InstanceFault.__table__"},{"line_number":6036,"context_line":"        default_deleted \u003d faults_tbl.c.deleted.default.arg"},{"line_number":6037,"context_line":"        latest_faults \u003d sql.select(["},{"line_number":6038,"context_line":"            faults_tbl.c.instance_uuid,"},{"line_number":6039,"context_line":"            sql.func.max(faults_tbl.c.id).label(\u0027max_id\u0027),"},{"line_number":6040,"context_line":"        ]).where("}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6eadb0_79405657","line":6037,"updated":"2016-12-13 11:10:08.000000000","message":"I think you could use model_query() here to let it deal with filtering by `deleted`. It would look like:\n\n        faults_tbl \u003d models.InstanceFault.__table__\n        latest_faults \u003d model_query(\n            context, models.InstanceFault,\n            [faults_tbl.c.instance_uuid,\n             sql.func.max(faults_tbl.c.id).label(\u0027max_id\u0027)],\n            read_deleted\u003d\u0027no\u0027\n        ).filter(\n            faults_tbl.c.instance_uuid.in_(instance_uuids)\n        ).group_by(\n            faults_tbl.c.instance_uuid\n        ).subquery(name\u003d\"latest_faults\")\n\n\nI checked the generated SQL and it corresponds to what you expect above:\n\nSELECT instance_faults.created_at,\n       instance_faults.updated_at,\n       instance_faults.deleted_at,\n       instance_faults.deleted,\n       instance_faults.id,\n       instance_faults.instance_uuid,\n       instance_faults.code,\n       instance_faults.message,\n       instance_faults.details,\n       instance_faults.host\nFROM instance_faults\nJOIN\n  (SELECT instance_faults.instance_uuid AS instance_uuid,\n          max(instance_faults.id) AS max_id\n   FROM instance_faults\n   WHERE instance_faults.deleted \u003d :deleted_1\n     AND instance_faults.instance_uuid IN (:instance_uuid_1,\n                                           :instance_uuid_2)\n   GROUP BY instance_faults.instance_uuid) AS latest_faults ON instance_faults.id \u003d latest_faults.max_id","commit_id":"9be3ea227d3249042f849b7f92cab7f2c7f6b14e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"4ddd5b861bbdb65e4e7ffe6d37b32b49af3f81be","unresolved":false,"context_lines":[{"line_number":6053,"context_line":""},{"line_number":6054,"context_line":"        query \u003d sql.select([faults_tbl]).select_from(join)"},{"line_number":6055,"context_line":"        engine \u003d get_engine(context)"},{"line_number":6056,"context_line":"        conn \u003d engine.connect()"},{"line_number":6057,"context_line":"        rows \u003d conn.execute(query).fetchall()"},{"line_number":6058,"context_line":""},{"line_number":6059,"context_line":"    output \u003d {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6eadb0_e0fcaf40","line":6056,"updated":"2016-12-13 11:10:08.000000000","message":"No need to get an Engine instance and open a new connection, you could use a Session injected by enginefacade:\n\n   rows \u003d context.session.execute(query).fetchall()\n\nThis way instance_fault_get_by_instance_uuids() will participate in an ongoing DB transaction (if there is one)","commit_id":"9be3ea227d3249042f849b7f92cab7f2c7f6b14e"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"036ff9e469a9281517cc703089c9d51d6b0414e6","unresolved":false,"context_lines":[{"line_number":6086,"context_line":"        )"},{"line_number":6087,"context_line":""},{"line_number":6088,"context_line":"        query \u003d sql.select([faults_tbl]).select_from(join)"},{"line_number":6089,"context_line":"        rows \u003d context.session.execute(query).fetchall()"},{"line_number":6090,"context_line":""},{"line_number":6091,"context_line":"    output \u003d {}"},{"line_number":6092,"context_line":"    for instance_uuid in instance_uuids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_40d4361f","line":6089,"updated":"2016-12-14 15:45:39.000000000","message":"The 2 sides of this branch are very different, even though they\u0027re returning the same data, just filtered differently.\n\nHow about something like this instead: http://paste.openstack.org/show/592359/","commit_id":"5ea4031516e3b575b344a3be3d64c10d9f193097"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":6038,"context_line":"@pick_context_manager_reader"},{"line_number":6039,"context_line":"def instance_fault_get_by_instance_uuids(context, instance_uuids,"},{"line_number":6040,"context_line":"                                         latest\u003dFalse):"},{"line_number":6041,"context_line":"    \"\"\"Get all instance faults for the provided instance_uuids."},{"line_number":6042,"context_line":""},{"line_number":6043,"context_line":"    :param instance_uuids: List of UUIDs of instances to grab faults for"},{"line_number":6044,"context_line":"    :param latest: Optional boolean indicating we should only return the latest"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_41af55e3","line":6041,"range":{"start_line":6041,"start_character":11,"end_line":6041,"end_character":14},"updated":"2016-12-29 16:40:05.000000000","message":"nit: maybe remove \u0027all\u0027 here since if latest\u003dTrue we don\u0027t return \u0027all\u0027.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":6042,"context_line":""},{"line_number":6043,"context_line":"    :param instance_uuids: List of UUIDs of instances to grab faults for"},{"line_number":6044,"context_line":"    :param latest: Optional boolean indicating we should only return the latest"},{"line_number":6045,"context_line":"                   fault for the instance"},{"line_number":6046,"context_line":"    \"\"\""},{"line_number":6047,"context_line":"    if not instance_uuids:"},{"line_number":6048,"context_line":"        return {}"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_01a54d03","line":6045,"range":{"start_line":6045,"start_character":29,"end_line":6045,"end_character":32},"updated":"2016-12-29 16:40:05.000000000","message":"nit: s/the/each/","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":6085,"context_line":"                           faults_tbl.c.id \u003d\u003d latest_faults.c.max_id)"},{"line_number":6086,"context_line":"    else:"},{"line_number":6087,"context_line":"        query \u003d query.filter(models.InstanceFault.instance_uuid.in_("},{"line_number":6088,"context_line":"                                        instance_uuids)).order_by(desc(\"id\"))"},{"line_number":6089,"context_line":""},{"line_number":6090,"context_line":"    output \u003d {}"},{"line_number":6091,"context_line":"    for instance_uuid in instance_uuids:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_41967595","line":6088,"range":{"start_line":6088,"start_character":57,"end_line":6088,"end_character":77},"updated":"2016-12-29 16:40:05.000000000","message":"Why does this drop the created_at filter?\n\norder_by(desc(\"created_at\"), desc(\"id\"))\n\nBecause it\u0027s redundant?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d2e7ada161964f452cf54156b3b50f862ce06b0b","unresolved":false,"context_lines":[{"line_number":6085,"context_line":"                           faults_tbl.c.id \u003d\u003d latest_faults.c.max_id)"},{"line_number":6086,"context_line":"    else:"},{"line_number":6087,"context_line":"        query \u003d query.filter(models.InstanceFault.instance_uuid.in_("},{"line_number":6088,"context_line":"                                        instance_uuids)).order_by(desc(\"id\"))"},{"line_number":6089,"context_line":""},{"line_number":6090,"context_line":"    output \u003d {}"},{"line_number":6091,"context_line":"    for instance_uuid in instance_uuids:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_c18ee526","line":6088,"range":{"start_line":6088,"start_character":57,"end_line":6088,"end_character":77},"in_reply_to":"ba5201f7_41967595","updated":"2016-12-29 16:50:34.000000000","message":"Correct, it\u0027s redundant and we have an index on id, not on created_at.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":6092,"context_line":"        output[instance_uuid] \u003d []"},{"line_number":6093,"context_line":""},{"line_number":6094,"context_line":"    for row in query:"},{"line_number":6095,"context_line":"        output[row.instance_uuid].append(row._asdict())"},{"line_number":6096,"context_line":""},{"line_number":6097,"context_line":"    return output"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_a18d617d","line":6094,"range":{"start_line":6094,"start_character":15,"end_line":6094,"end_character":20},"updated":"2016-12-29 16:40:05.000000000","message":"Why didn\u0027t we have to call all() on the query to execute it and get results back?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d2e7ada161964f452cf54156b3b50f862ce06b0b","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":6092,"context_line":"        output[instance_uuid] \u003d []"},{"line_number":6093,"context_line":""},{"line_number":6094,"context_line":"    for row in query:"},{"line_number":6095,"context_line":"        output[row.instance_uuid].append(row._asdict())"},{"line_number":6096,"context_line":""},{"line_number":6097,"context_line":"    return output"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_a1aac16e","line":6094,"range":{"start_line":6094,"start_character":15,"end_line":6094,"end_character":20},"in_reply_to":"ba5201f7_a18d617d","updated":"2016-12-29 16:50:34.000000000","message":"Because the result of model_query() is a Query object and calling Query.__iter__ results in executing the query and returning an iterator ResultProxy object.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8147691c39bcbedf210f08d4b55d09ea85ecad35","unresolved":false,"context_lines":[{"line_number":6091,"context_line":"    for instance_uuid in instance_uuids:"},{"line_number":6092,"context_line":"        output[instance_uuid] \u003d []"},{"line_number":6093,"context_line":""},{"line_number":6094,"context_line":"    for row in query:"},{"line_number":6095,"context_line":"        output[row.instance_uuid].append(row._asdict())"},{"line_number":6096,"context_line":""},{"line_number":6097,"context_line":"    return output"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_21bb3174","line":6094,"range":{"start_line":6094,"start_character":15,"end_line":6094,"end_character":20},"in_reply_to":"ba5201f7_a1aac16e","updated":"2016-12-29 17:06:38.000000000","message":"Yeah, we can skip creation of a list here (all() call) if we want to process (i.e. iterate over) the rows first before returning them.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":6092,"context_line":"        output[instance_uuid] \u003d []"},{"line_number":6093,"context_line":""},{"line_number":6094,"context_line":"    for row in query:"},{"line_number":6095,"context_line":"        output[row.instance_uuid].append(row._asdict())"},{"line_number":6096,"context_line":""},{"line_number":6097,"context_line":"    return output"},{"line_number":6098,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_e1eda926","line":6095,"range":{"start_line":6095,"start_character":45,"end_line":6095,"end_character":52},"updated":"2016-12-29 16:40:05.000000000","message":"I\u0027m not sure we should be relying on an internal method of sqlalchemy in case it changes, what was wrong with just using dict(row)?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8147691c39bcbedf210f08d4b55d09ea85ecad35","unresolved":false,"context_lines":[{"line_number":6092,"context_line":"        output[instance_uuid] \u003d []"},{"line_number":6093,"context_line":""},{"line_number":6094,"context_line":"    for row in query:"},{"line_number":6095,"context_line":"        output[row.instance_uuid].append(row._asdict())"},{"line_number":6096,"context_line":""},{"line_number":6097,"context_line":"    return output"},{"line_number":6098,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_a19001ee","line":6095,"range":{"start_line":6095,"start_character":45,"end_line":6095,"end_character":52},"in_reply_to":"ba5201f7_c1dc251b","updated":"2016-12-29 17:06:38.000000000","message":"It was me. That\u0027s the difference between using of Query and execute() in SQLAlchemy: we get named tuples instead of dict-like objects. Unfortunately, dict() won\u0027t work on named tuples, but we\u0027ve got _asdict() to help. My understanding that it\u0027s not really internal - https://docs.python.org/2/library/collections.html#collections.somenamedtuple._asdict - it just has a prefix to distinguish method names from tuple attribute names.\n\nNow that we skip the creation of ORM objects, we save some memory and CPU cycles (and we used to throw created objects away immediately and converted them to plain dict anyway).","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d2e7ada161964f452cf54156b3b50f862ce06b0b","unresolved":false,"context_lines":[{"line_number":6092,"context_line":"        output[instance_uuid] \u003d []"},{"line_number":6093,"context_line":""},{"line_number":6094,"context_line":"    for row in query:"},{"line_number":6095,"context_line":"        output[row.instance_uuid].append(row._asdict())"},{"line_number":6096,"context_line":""},{"line_number":6097,"context_line":"    return output"},{"line_number":6098,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_c1dc251b","line":6095,"range":{"start_line":6095,"start_character":45,"end_line":6095,"end_character":52},"in_reply_to":"ba5201f7_e1eda926","updated":"2016-12-29 16:50:34.000000000","message":"Not sure who changed that. mdbooth?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"}],"nova/objects/instance.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":1160,"context_line":"        # Build an instance_uuid:latest-fault mapping"},{"line_number":1161,"context_line":"        expected_attrs.remove(\u0027fault\u0027)"},{"line_number":1162,"context_line":"        instance_uuids \u003d [inst[\u0027uuid\u0027] for inst in db_inst_list]"},{"line_number":1163,"context_line":"        faults \u003d objects.InstanceFaultList.get_by_instance_uuids("},{"line_number":1164,"context_line":"            context, instance_uuids)"},{"line_number":1165,"context_line":"        for fault in faults:"},{"line_number":1166,"context_line":"            if fault.instance_uuid not in inst_faults:"},{"line_number":1167,"context_line":"                inst_faults[fault.instance_uuid] \u003d fault"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_41f915c8","line":1164,"range":{"start_line":1163,"start_character":8,"end_line":1164,"end_character":36},"updated":"2016-12-29 16:40:05.000000000","message":"Do we want to use the new method here too?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e591451dd3e275ff1b8168c8f95108b700c3d0b2","unresolved":false,"context_lines":[{"line_number":1160,"context_line":"        # Build an instance_uuid:latest-fault mapping"},{"line_number":1161,"context_line":"        expected_attrs.remove(\u0027fault\u0027)"},{"line_number":1162,"context_line":"        instance_uuids \u003d [inst[\u0027uuid\u0027] for inst in db_inst_list]"},{"line_number":1163,"context_line":"        faults \u003d objects.InstanceFaultList.get_by_instance_uuids("},{"line_number":1164,"context_line":"            context, instance_uuids)"},{"line_number":1165,"context_line":"        for fault in faults:"},{"line_number":1166,"context_line":"            if fault.instance_uuid not in inst_faults:"},{"line_number":1167,"context_line":"                inst_faults[fault.instance_uuid] \u003d fault"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_9cbb620e","line":1164,"range":{"start_line":1163,"start_character":8,"end_line":1164,"end_character":36},"in_reply_to":"ba5201f7_41f915c8","updated":"2016-12-29 17:33:39.000000000","message":"We could handle this separately if needed.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":1342,"context_line":"        :returns: A list of instance uuids for which faults were found."},{"line_number":1343,"context_line":"        \"\"\""},{"line_number":1344,"context_line":"        uuids \u003d [inst.uuid for inst in self]"},{"line_number":1345,"context_line":"        faults \u003d objects.InstanceFaultList.get_latest_by_instance_uuids("},{"line_number":1346,"context_line":"            self._context, uuids)"},{"line_number":1347,"context_line":"        faults_by_uuid \u003d {}"},{"line_number":1348,"context_line":"        for fault in faults:"},{"line_number":1349,"context_line":"            faults_by_uuid[fault.instance_uuid] \u003d fault"},{"line_number":1350,"context_line":""},{"line_number":1351,"context_line":"        for instance in self:"},{"line_number":1352,"context_line":"            if instance.uuid in faults_by_uuid:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_01ab8d8a","line":1349,"range":{"start_line":1345,"start_character":8,"end_line":1349,"end_character":55},"updated":"2016-12-29 16:40:05.000000000","message":"If the DB API we used before used to return the instance faults list keyed by instance uuid but in descending order based on when the fault was created, when we iterate over the faults and store them in faults_by_uuid, wouldn\u0027t the fault that gets stored for a given instance be the oldest created fault? If so, wouldn\u0027t we have now changed this logic to store the newest fault per instance?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d2e7ada161964f452cf54156b3b50f862ce06b0b","unresolved":false,"context_lines":[{"line_number":1342,"context_line":"        :returns: A list of instance uuids for which faults were found."},{"line_number":1343,"context_line":"        \"\"\""},{"line_number":1344,"context_line":"        uuids \u003d [inst.uuid for inst in self]"},{"line_number":1345,"context_line":"        faults \u003d objects.InstanceFaultList.get_latest_by_instance_uuids("},{"line_number":1346,"context_line":"            self._context, uuids)"},{"line_number":1347,"context_line":"        faults_by_uuid \u003d {}"},{"line_number":1348,"context_line":"        for fault in faults:"},{"line_number":1349,"context_line":"            faults_by_uuid[fault.instance_uuid] \u003d fault"},{"line_number":1350,"context_line":""},{"line_number":1351,"context_line":"        for instance in self:"},{"line_number":1352,"context_line":"            if instance.uuid in faults_by_uuid:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_81dbfd1a","line":1349,"range":{"start_line":1345,"start_character":8,"end_line":1349,"end_character":55},"in_reply_to":"ba5201f7_01ab8d8a","updated":"2016-12-29 16:50:34.000000000","message":"We only return a single fault per instance UUID (the newest fault) now from InstanceFaultList.get_latest_by_instance_uuids().","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e591451dd3e275ff1b8168c8f95108b700c3d0b2","unresolved":false,"context_lines":[{"line_number":1342,"context_line":"        :returns: A list of instance uuids for which faults were found."},{"line_number":1343,"context_line":"        \"\"\""},{"line_number":1344,"context_line":"        uuids \u003d [inst.uuid for inst in self]"},{"line_number":1345,"context_line":"        faults \u003d objects.InstanceFaultList.get_latest_by_instance_uuids("},{"line_number":1346,"context_line":"            self._context, uuids)"},{"line_number":1347,"context_line":"        faults_by_uuid \u003d {}"},{"line_number":1348,"context_line":"        for fault in faults:"},{"line_number":1349,"context_line":"            faults_by_uuid[fault.instance_uuid] \u003d fault"},{"line_number":1350,"context_line":""},{"line_number":1351,"context_line":"        for instance in self:"},{"line_number":1352,"context_line":"            if instance.uuid in faults_by_uuid:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_1caa12d5","line":1349,"range":{"start_line":1345,"start_character":8,"end_line":1349,"end_character":55},"in_reply_to":"ba5201f7_81dbfd1a","updated":"2016-12-29 17:33:39.000000000","message":"Yeah, plus I was totally missing \"if fault.instance_uuid not in faults_by_uuid:\" in the old logic such that it would only store the latest fault and ignore the rest.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"}],"nova/objects/instance_fault.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"036ff9e469a9281517cc703089c9d51d6b0414e6","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                                                               latest\u003dTrue)"},{"line_number":111,"context_line":"        db_faultlist \u003d itertools.chain(*db_faultdict.values())"},{"line_number":112,"context_line":"        return base.obj_make_list(context, cls(context), objects.InstanceFault,"},{"line_number":113,"context_line":"                                  db_faultlist)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    @base.remotable_classmethod"},{"line_number":116,"context_line":"    def get_by_instance_uuids(cls, context, instance_uuids):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_5aa263af","line":113,"updated":"2016-12-14 15:45:39.000000000","message":"I\u0027m assuming that we can\u0027t directly return a dict here? It\u0027s kinda weird that we get a dict, we turn it into a list, then the only caller turns it back into an identically-keyed dict.","commit_id":"5ea4031516e3b575b344a3be3d64c10d9f193097"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    # Version 1.0: Initial version"},{"line_number":97,"context_line":"    #              InstanceFault \u003c\u003d version 1.1"},{"line_number":98,"context_line":"    # Version 1.1: InstanceFault version 1.2"},{"line_number":99,"context_line":"    # Version 1.2: Added get_latest_by_instance_uuids() method"},{"line_number":100,"context_line":"    VERSION \u003d \u00271.2\u0027"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_4172f57d","line":99,"updated":"2016-12-29 16:40:05.000000000","message":"If this bug is pretty bad for performance (at least on those that don\u0027t regularly prune their database), then it\u0027d be nice to backport the fix, but we can\u0027t do that with an object version bump.\n\nWould it be possible to fix the bug with a partial tactical fix which can be backported, and then do this long-term strategic fix which has the object version bump and the new remotable method?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"64079912305c69166d7eef89f229f6b6d94bbb76","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    # Version 1.0: Initial version"},{"line_number":97,"context_line":"    #              InstanceFault \u003c\u003d version 1.1"},{"line_number":98,"context_line":"    # Version 1.1: InstanceFault version 1.2"},{"line_number":99,"context_line":"    # Version 1.2: Added get_latest_by_instance_uuids() method"},{"line_number":100,"context_line":"    VERSION \u003d \u00271.2\u0027"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_1c877210","line":99,"in_reply_to":"ba5201f7_3c96f694","updated":"2016-12-29 17:54:38.000000000","message":"Actually I don\u0027t think I can do that without changing InstanceList.fill_faults to call db.instance_fault_get_by_instance_uuids directly, which then makes it a direct-to-db call which is not cool. Calling that from nova-api is fine since there are other things in the API which hit the database directly still, but if anything else creates an InstanceList with \u0027fault\u0027 in expected_attrs and doesn\u0027t have a database connection, like nova-compute, then that\u0027s going to explode. Chances are slim there is anything like that though...but still seems risky just for backporting what is a super latent performance bug.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e591451dd3e275ff1b8168c8f95108b700c3d0b2","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    # Version 1.0: Initial version"},{"line_number":97,"context_line":"    #              InstanceFault \u003c\u003d version 1.1"},{"line_number":98,"context_line":"    # Version 1.1: InstanceFault version 1.2"},{"line_number":99,"context_line":"    # Version 1.2: Added get_latest_by_instance_uuids() method"},{"line_number":100,"context_line":"    VERSION \u003d \u00271.2\u0027"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_3c96f694","line":99,"in_reply_to":"ba5201f7_4172f57d","updated":"2016-12-29 17:33:39.000000000","message":"I\u0027m thinking we could create a backportable fix by making the same DB API changes you have here and adding the \u0027latest\u0027 kwarg to the DB API method, and change InstanceList.fill_faults to use that (which isn\u0027t remotable), and then we can backport that guy. Then in a follow up fix which we don\u0027t backport, we add this new remotable method to InstanceFaultList and change fill_faults to use that (like you have in this patch).","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07f02bb508b91922bd34fbea8fc18ea2640bf289","unresolved":false,"context_lines":[{"line_number":113,"context_line":"                                  db_faultlist)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    @base.remotable_classmethod"},{"line_number":116,"context_line":"    def get_by_instance_uuids(cls, context, instance_uuids):"},{"line_number":117,"context_line":"        db_faultdict \u003d db.instance_fault_get_by_instance_uuids(context,"},{"line_number":118,"context_line":"                                                               instance_uuids)"},{"line_number":119,"context_line":"        db_faultlist \u003d itertools.chain(*db_faultdict.values())"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_c10425c8","line":116,"range":{"start_line":116,"start_character":8,"end_line":116,"end_character":29},"updated":"2016-12-29 16:40:05.000000000","message":"If this is no longer used anywhere we might want to add a comment that it\u0027s deprecated and we should remove it in version 2.0 of this object.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c9e67f19736f22599ec5d83157600eeac88a45db","unresolved":false,"context_lines":[{"line_number":113,"context_line":"                                  db_faultlist)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    @base.remotable_classmethod"},{"line_number":116,"context_line":"    def get_by_instance_uuids(cls, context, instance_uuids):"},{"line_number":117,"context_line":"        db_faultdict \u003d db.instance_fault_get_by_instance_uuids(context,"},{"line_number":118,"context_line":"                                                               instance_uuids)"},{"line_number":119,"context_line":"        db_faultlist \u003d itertools.chain(*db_faultdict.values())"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_6056a2cf","line":116,"range":{"start_line":116,"start_character":8,"end_line":116,"end_character":29},"in_reply_to":"ba5201f7_7c907e96","updated":"2017-01-04 18:02:56.000000000","message":"+1 for follow up clean up.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e591451dd3e275ff1b8168c8f95108b700c3d0b2","unresolved":false,"context_lines":[{"line_number":113,"context_line":"                                  db_faultlist)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    @base.remotable_classmethod"},{"line_number":116,"context_line":"    def get_by_instance_uuids(cls, context, instance_uuids):"},{"line_number":117,"context_line":"        db_faultdict \u003d db.instance_fault_get_by_instance_uuids(context,"},{"line_number":118,"context_line":"                                                               instance_uuids)"},{"line_number":119,"context_line":"        db_faultlist \u003d itertools.chain(*db_faultdict.values())"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_7c907e96","line":116,"range":{"start_line":116,"start_character":8,"end_line":116,"end_character":29},"in_reply_to":"ba5201f7_c10425c8","updated":"2016-12-29 17:33:39.000000000","message":"Looks like it is still used in one place when constructing an instance list if \u0027fault\u0027 is in expected_attrs. That could probably be cleaned up in a follow up patch.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"}],"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":"e591451dd3e275ff1b8168c8f95108b700c3d0b2","unresolved":false,"context_lines":[{"line_number":4203,"context_line":"                fault \u003d db.instance_fault_create(self.ctxt, fault_values)"},{"line_number":4204,"context_line":"                expected[uuid].append(fault)"},{"line_number":4205,"context_line":""},{"line_number":4206,"context_line":"        # We are only interested in the latest fault for each instance"},{"line_number":4207,"context_line":"        for uuid in expected:"},{"line_number":4208,"context_line":"            expected[uuid] \u003d expected[uuid][-1:]"},{"line_number":4209,"context_line":""},{"line_number":4210,"context_line":"        # Ensure faults are saved"},{"line_number":4211,"context_line":"        faults \u003d db.instance_fault_get_by_instance_uuids(self.ctxt, uuids,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_dcd40a49","line":4208,"range":{"start_line":4206,"start_character":8,"end_line":4208,"end_character":48},"updated":"2016-12-29 17:33:39.000000000","message":"See https://review.openstack.org/#/c/415736/ we could just do something similar there.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e591451dd3e275ff1b8168c8f95108b700c3d0b2","unresolved":false,"context_lines":[{"line_number":4212,"context_line":"                                                         latest\u003dTrue)"},{"line_number":4213,"context_line":"        self.assertEqual(len(expected), len(faults))"},{"line_number":4214,"context_line":"        for uuid in uuids:"},{"line_number":4215,"context_line":"            self._assertEqualListsOfObjects(expected[uuid], faults[uuid])"},{"line_number":4216,"context_line":""},{"line_number":4217,"context_line":"    def test_instance_faults_get_by_instance_uuids_no_faults(self):"},{"line_number":4218,"context_line":"        uuid \u003d uuidsentinel.uuid1"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_fcc9cead","line":4215,"range":{"start_line":4215,"start_character":17,"end_line":4215,"end_character":43},"updated":"2016-12-29 17:33:39.000000000","message":"Again see https://review.openstack.org/#/c/415736/ - this sorts the lists before comparing them. Since the list should be length 1 that shouldn\u0027t be an issue, but shouldn\u0027t we then assert that len(faults) \u003d\u003d 1?","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3a3180d14008da0ff5f8bbe8e89daf7b3ab6eeb5","unresolved":false,"context_lines":[{"line_number":4212,"context_line":"                                                         latest\u003dTrue)"},{"line_number":4213,"context_line":"        self.assertEqual(len(expected), len(faults))"},{"line_number":4214,"context_line":"        for uuid in uuids:"},{"line_number":4215,"context_line":"            self._assertEqualListsOfObjects(expected[uuid], faults[uuid])"},{"line_number":4216,"context_line":""},{"line_number":4217,"context_line":"    def test_instance_faults_get_by_instance_uuids_no_faults(self):"},{"line_number":4218,"context_line":"        uuid \u003d uuidsentinel.uuid1"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_1c9e9256","line":4215,"range":{"start_line":4215,"start_character":17,"end_line":4215,"end_character":43},"in_reply_to":"ba5201f7_fcc9cead","updated":"2016-12-29 17:57:02.000000000","message":"Nevermind [-1:] makes it a single item list.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"}],"nova/tests/unit/objects/test_instance.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"716a207ec4efd25f4787a192479ea0d88f1c6485","unresolved":false,"context_lines":[{"line_number":1808,"context_line":"        for inst in insts:"},{"line_number":1809,"context_line":"            inst.obj_reset_changes()"},{"line_number":1810,"context_line":"        db_faults \u003d {"},{"line_number":1811,"context_line":"            \u0027uuid1\u0027: [{\u0027id\u0027: 123,"},{"line_number":1812,"context_line":"                       \u0027instance_uuid\u0027: uuids.db_fault_1,"},{"line_number":1813,"context_line":"                       \u0027code\u0027: 456,"},{"line_number":1814,"context_line":"                       \u0027message\u0027: \u0027Fake message\u0027,"},{"line_number":1815,"context_line":"                       \u0027details\u0027: \u0027No details\u0027,"},{"line_number":1816,"context_line":"                       \u0027host\u0027: \u0027foo\u0027,"},{"line_number":1817,"context_line":"                       \u0027deleted\u0027: False,"},{"line_number":1818,"context_line":"                       \u0027deleted_at\u0027: None,"},{"line_number":1819,"context_line":"                       \u0027updated_at\u0027: None,"},{"line_number":1820,"context_line":"                       \u0027created_at\u0027: None,"},{"line_number":1821,"context_line":"                       }"},{"line_number":1822,"context_line":"                      ]}"},{"line_number":1823,"context_line":"        mock_fault_get.return_value \u003d db_faults"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"        inst_list \u003d objects.InstanceList()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_6179b92f","line":1822,"range":{"start_line":1811,"start_character":21,"end_line":1822,"end_character":23},"updated":"2016-12-29 16:44:46.000000000","message":"Wouldn\u0027t this test be more interesting if we actually had more than one fault here? I guess with the change that\u0027s made, this would actually not have multiples because we only get the latest fault per instance now.","commit_id":"176c5c8a65efbde01020bc69a97bb7d05720589e"}]}