)]}'
{"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":1944,"context_line":""},{"line_number":1945,"context_line":"    :param context: security context"},{"line_number":1946,"context_line":"    :param instances: list of instances to fill"},{"line_number":1947,"context_line":"    :param manual_joins: list of tables to manually join (can be any"},{"line_number":1948,"context_line":"                         combination of \u0027metadata\u0027 and \u0027system_metadata\u0027 or"},{"line_number":1949,"context_line":"                         None to take the default of both)"},{"line_number":1950,"context_line":"    \"\"\""},{"line_number":1951,"context_line":"    uuids \u003d [inst[\u0027uuid\u0027] for inst in instances]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_1d4907b7","line":1948,"range":{"start_line":1947,"start_character":57,"end_line":1948,"end_character":72},"updated":"2019-12-05 14:24:21.000000000","message":"Well this is clearly out of date but it was before your change as well.","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"16cf41cd5e165600d27b45c37ccf588715122ef8","unresolved":false,"context_lines":[{"line_number":1944,"context_line":""},{"line_number":1945,"context_line":"    :param context: security context"},{"line_number":1946,"context_line":"    :param instances: list of instances to fill"},{"line_number":1947,"context_line":"    :param manual_joins: list of tables to manually join (can be any"},{"line_number":1948,"context_line":"                         combination of \u0027metadata\u0027 and \u0027system_metadata\u0027 or"},{"line_number":1949,"context_line":"                         None to take the default of both)"},{"line_number":1950,"context_line":"    \"\"\""},{"line_number":1951,"context_line":"    uuids \u003d [inst[\u0027uuid\u0027] for inst in instances]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_4d1306b9","line":1948,"range":{"start_line":1947,"start_character":57,"end_line":1948,"end_character":72},"in_reply_to":"3fa7e38b_1d4907b7","updated":"2019-12-11 07:22:13.000000000","message":"listed all tables to join.","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":1982,"context_line":"    filled_instances \u003d []"},{"line_number":1983,"context_line":"    for inst in instances:"},{"line_number":1984,"context_line":"        inst \u003d dict(inst)"},{"line_number":1985,"context_line":"        if \u0027is_volume_backed\u0027 in manual_joins:"},{"line_number":1986,"context_line":"            inst[\u0027is_volume_backed\u0027] \u003d inst[\u0027uuid\u0027] in bfv_instances_uuids"},{"line_number":1987,"context_line":"        inst[\u0027system_metadata\u0027] \u003d sys_meta[inst[\u0027uuid\u0027]]"},{"line_number":1988,"context_line":"        inst[\u0027metadata\u0027] \u003d meta[inst[\u0027uuid\u0027]]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_5dd8dfee","line":1985,"updated":"2019-12-05 14:24:21.000000000","message":"At first I was going to say this isn\u0027t necessary since if it\u0027s False then \"inst[\u0027uuid\u0027] in bfv_instances_uuids\" would just be False, but I guess the idea is that if you\u0027re not asking for the is_volume_backed flag, setting False might be misleading if the instance is actually volume-backed but we didn\u0027t do the work to check.","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":1995,"context_line":"    return filled_instances"},{"line_number":1996,"context_line":""},{"line_number":1997,"context_line":""},{"line_number":1998,"context_line":"def _get_volume_backed_instances(context, instance_uuids):"},{"line_number":1999,"context_line":"    uuids \u003d []"},{"line_number":2000,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2001,"context_line":"                        (models.Instance.uuid,),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_9de3b7a0","line":1998,"range":{"start_line":1998,"start_character":42,"end_line":1998,"end_character":56},"updated":"2019-12-05 14:24:21.000000000","message":"_instance_system_metadata_get_multi short circuits if this is empty, we should probably do the same here (to avoid a query which won\u0027t have any results).","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"16cf41cd5e165600d27b45c37ccf588715122ef8","unresolved":false,"context_lines":[{"line_number":1995,"context_line":"    return filled_instances"},{"line_number":1996,"context_line":""},{"line_number":1997,"context_line":""},{"line_number":1998,"context_line":"def _get_volume_backed_instances(context, instance_uuids):"},{"line_number":1999,"context_line":"    uuids \u003d []"},{"line_number":2000,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2001,"context_line":"                        (models.Instance.uuid,),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_0d098e82","line":1998,"range":{"start_line":1998,"start_character":42,"end_line":1998,"end_character":56},"in_reply_to":"3fa7e38b_9de3b7a0","updated":"2019-12-11 07:22:13.000000000","message":"Done","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":1999,"context_line":"    uuids \u003d []"},{"line_number":2000,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2001,"context_line":"                        (models.Instance.uuid,),"},{"line_number":2002,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2003,"context_line":"                        ).join(("},{"line_number":2004,"context_line":"        models.BlockDeviceMapping, models.Instance.uuid \u003d\u003d"},{"line_number":2005,"context_line":"        models.BlockDeviceMapping.instance_uuid)).filter(and_(("}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_9d98971a","line":2002,"range":{"start_line":2002,"start_character":24,"end_line":2002,"end_character":42},"updated":"2019-12-05 14:24:21.000000000","message":"I wasn\u0027t sure about this but _instance_system_metadata_get_multi does it too so looks like it\u0027s OK, and makes sense if we\u0027re querying for deleted instances.","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":2002,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2003,"context_line":"                        ).join(("},{"line_number":2004,"context_line":"        models.BlockDeviceMapping, models.Instance.uuid \u003d\u003d"},{"line_number":2005,"context_line":"        models.BlockDeviceMapping.instance_uuid)).filter(and_(("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0),"},{"line_number":2007,"context_line":"        (models.Instance.uuid.in_(instance_uuids)),"},{"line_number":2008,"context_line":"        (models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":"    for instance in query.all():"},{"line_number":2011,"context_line":"        uuids.append(instance[0])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_9db17794","line":2008,"range":{"start_line":2005,"start_character":57,"end_line":2008,"end_character":65},"updated":"2019-12-05 14:24:21.000000000","message":"The indenting here makes this is a bit weird to read - can\u0027t we align all 3 parts of the AND clause?","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"16cf41cd5e165600d27b45c37ccf588715122ef8","unresolved":false,"context_lines":[{"line_number":2002,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2003,"context_line":"                        ).join(("},{"line_number":2004,"context_line":"        models.BlockDeviceMapping, models.Instance.uuid \u003d\u003d"},{"line_number":2005,"context_line":"        models.BlockDeviceMapping.instance_uuid)).filter(and_(("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0),"},{"line_number":2007,"context_line":"        (models.Instance.uuid.in_(instance_uuids)),"},{"line_number":2008,"context_line":"        (models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":"    for instance in query.all():"},{"line_number":2011,"context_line":"        uuids.append(instance[0])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_cd261617","line":2008,"range":{"start_line":2005,"start_character":57,"end_line":2008,"end_character":65},"in_reply_to":"3fa7e38b_9db17794","updated":"2019-12-11 07:22:13.000000000","message":"Done","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":2007,"context_line":"        (models.Instance.uuid.in_(instance_uuids)),"},{"line_number":2008,"context_line":"        (models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":"    for instance in query.all():"},{"line_number":2011,"context_line":"        uuids.append(instance[0])"},{"line_number":2012,"context_line":""},{"line_number":2013,"context_line":"    return uuids"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_1d8ca7d6","line":2010,"range":{"start_line":2010,"start_character":8,"end_line":2010,"end_character":16},"updated":"2019-12-05 14:24:21.000000000","message":"nit: name this instance_uuid since that\u0027s what it is","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"16cf41cd5e165600d27b45c37ccf588715122ef8","unresolved":false,"context_lines":[{"line_number":2007,"context_line":"        (models.Instance.uuid.in_(instance_uuids)),"},{"line_number":2008,"context_line":"        (models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":"    for instance in query.all():"},{"line_number":2011,"context_line":"        uuids.append(instance[0])"},{"line_number":2012,"context_line":""},{"line_number":2013,"context_line":"    return uuids"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_ed2192fd","line":2010,"range":{"start_line":2010,"start_character":8,"end_line":2010,"end_character":16},"in_reply_to":"3fa7e38b_1d8ca7d6","updated":"2019-12-11 07:22:13.000000000","message":"Done","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":1948,"context_line":"    :param manual_joins: list of tables to manually join(can be any"},{"line_number":1949,"context_line":"                         combination of \u0027metadata\u0027, \u0027system_metadata\u0027,"},{"line_number":1950,"context_line":"                         \u0027pci_devices\u0027, \u0027fault\u0027, \u0027is_volume_backed\u0027 or"},{"line_number":1951,"context_line":"                         None to take the default of both)"},{"line_number":1952,"context_line":"    \"\"\""},{"line_number":1953,"context_line":"    uuids \u003d [inst[\u0027uuid\u0027] for inst in instances]"},{"line_number":1954,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_644b4a86","line":1951,"range":{"start_line":1951,"start_character":50,"end_line":1951,"end_character":57},"updated":"2019-12-13 21:06:02.000000000","message":"This no longer makes sense. I\u0027d probably just avoid this kind of cleanup distraction in your series and fix it up separately later (or in a preceding patch which you\u0027d then build on) if you want to correct these docstrings.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":1948,"context_line":"    :param manual_joins: list of tables to manually join(can be any"},{"line_number":1949,"context_line":"                         combination of \u0027metadata\u0027, \u0027system_metadata\u0027,"},{"line_number":1950,"context_line":"                         \u0027pci_devices\u0027, \u0027fault\u0027, \u0027is_volume_backed\u0027 or"},{"line_number":1951,"context_line":"                         None to take the default of both)"},{"line_number":1952,"context_line":"    \"\"\""},{"line_number":1953,"context_line":"    uuids \u003d [inst[\u0027uuid\u0027] for inst in instances]"},{"line_number":1954,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c308c22f","line":1951,"range":{"start_line":1951,"start_character":50,"end_line":1951,"end_character":57},"in_reply_to":"3fa7e38b_644b4a86","updated":"2019-12-18 07:06:32.000000000","message":"keeping as is in this patch","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":2002,"context_line":"        return []"},{"line_number":2003,"context_line":""},{"line_number":2004,"context_line":"    uuids \u003d []"},{"line_number":2005,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2006,"context_line":"                        (models.Instance.uuid,),"},{"line_number":2007,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2008,"context_line":"                        ).join(("}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c4079e1e","line":2005,"updated":"2019-12-13 21:06:02.000000000","message":"Since Jay Pipes is no longer around we might want to have Mike Bayer take a look at this query just to make sure it\u0027s sufficiently efficient (as possible anyway). Mike is zzzeek in IRC.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"a1692441a58c2410cbbc5379bedde61479b7411e","unresolved":false,"context_lines":[{"line_number":2002,"context_line":"        return []"},{"line_number":2003,"context_line":""},{"line_number":2004,"context_line":"    uuids \u003d []"},{"line_number":2005,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2006,"context_line":"                        (models.Instance.uuid,),"},{"line_number":2007,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2008,"context_line":"                        ).join(("}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c7f240e5","line":2005,"in_reply_to":"3fa7e38b_c4079e1e","updated":"2019-12-13 21:20:37.000000000","message":"I can see there is a manual merge step implied by the above.   IIUC we are joining from \"instance\" to \"blockdevicemapping\" on \"instance_uuid\", essentially giving it the list of instance_uuids we already have and then essentially removing those for which the \"block device mapping\" criteria doesn\u0027t match.\n\nif BlockDeviceMapping.instance_uuid is a foreign key to Instance, that would mean you don\u0027t actually have to include the Instance table in this query, right?   however, this is not including whatever criteria \"model_query\" is adding so that might make this infeasible, if \"model_query\" needs to look at other columns on Instance and not BlockDeviceMapping.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb71f7d64a395068b4199bef466dababd9543c80","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2006,"context_line":"                        (models.Instance.uuid,),"},{"line_number":2007,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2008,"context_line":"                        ).join(("},{"line_number":2009,"context_line":"        models.BlockDeviceMapping,"},{"line_number":2010,"context_line":"        models.Instance.uuid \u003d\u003d models.BlockDeviceMapping.instance_uuid)"},{"line_number":2011,"context_line":"    ).filter(and_(("}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_67d26c36","line":2008,"range":{"start_line":2008,"start_character":26,"end_line":2008,"end_character":30},"updated":"2019-12-13 21:23:33.000000000","message":"We likely don\u0027t need this. We should just be able to do this:\n\nselect instance_uuid from bdms where instance_uuid in (instance_uuids) and boot_index\u003d0 and destination_type\u003d\u0027volume\u0027;\n\nright?\n\nAnd having said that, I don\u0027t think we need the model_query either - we\u0027re not scoping this to project or filtering out deleted bdms, so we should be able to just do this with straight sqlalchemy ORM queries and not use model_query or the join which would make this a lot better.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2006,"context_line":"                        (models.Instance.uuid,),"},{"line_number":2007,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2008,"context_line":"                        ).join(("},{"line_number":2009,"context_line":"        models.BlockDeviceMapping,"},{"line_number":2010,"context_line":"        models.Instance.uuid \u003d\u003d models.BlockDeviceMapping.instance_uuid)"},{"line_number":2011,"context_line":"    ).filter(and_(("}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_8302ca0b","line":2008,"range":{"start_line":2008,"start_character":26,"end_line":2008,"end_character":30},"in_reply_to":"3fa7e38b_67d26c36","updated":"2019-12-18 07:06:32.000000000","message":"Removed, done as suggested here.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"2fb46047a5748c79e79010b8ae213a4959c6c9c3","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"    query \u003d model_query(context, models.Instance,"},{"line_number":2006,"context_line":"                        (models.Instance.uuid,),"},{"line_number":2007,"context_line":"                        read_deleted\u003d\"yes\""},{"line_number":2008,"context_line":"                        ).join(("},{"line_number":2009,"context_line":"        models.BlockDeviceMapping,"},{"line_number":2010,"context_line":"        models.Instance.uuid \u003d\u003d models.BlockDeviceMapping.instance_uuid)"},{"line_number":2011,"context_line":"    ).filter(and_(("}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_3ee6b9e9","line":2008,"range":{"start_line":2008,"start_character":26,"end_line":2008,"end_character":30},"in_reply_to":"3fa7e38b_67d26c36","updated":"2019-12-14 16:14:24.000000000","message":"i don\u0027t know the model query part but what you have here doesn\u0027t need the instance table, so if you\u0027re going for a more efficient query you have all the wins.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4905a1a04cabc54545575e87eb3d5a80b79a5484","unresolved":false,"context_lines":[{"line_number":2002,"context_line":""},{"line_number":2003,"context_line":"    uuids \u003d []"},{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_7b5ac382","line":2005,"range":{"start_line":2005,"start_character":56,"end_line":2005,"end_character":57},"updated":"2019-12-18 22:49:55.000000000","message":"Is this...","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"b8676831d671aeb60132c36d3bb3ee01053e6bda","unresolved":false,"context_lines":[{"line_number":2002,"context_line":""},{"line_number":2003,"context_line":"    uuids \u003d []"},{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_84771408","line":2005,"range":{"start_line":2005,"start_character":56,"end_line":2005,"end_character":57},"in_reply_to":"3fa7e38b_7b5ac382","updated":"2019-12-19 16:35:38.000000000","message":"Done","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4905a1a04cabc54545575e87eb3d5a80b79a5484","unresolved":false,"context_lines":[{"line_number":2003,"context_line":"    uuids \u003d []"},{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_1b514f64","line":2006,"range":{"start_line":2006,"start_character":71,"end_line":2006,"end_character":72},"updated":"2019-12-18 22:49:55.000000000","message":"...and this really necessary?","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4905a1a04cabc54545575e87eb3d5a80b79a5484","unresolved":false,"context_lines":[{"line_number":2003,"context_line":"    uuids \u003d []"},{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_3b4c4bc8","line":2006,"range":{"start_line":2006,"start_character":74,"end_line":2006,"end_character":75},"updated":"2019-12-18 22:49:55.000000000","message":"same...","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"b8676831d671aeb60132c36d3bb3ee01053e6bda","unresolved":false,"context_lines":[{"line_number":2003,"context_line":"    uuids \u003d []"},{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_247e60e3","line":2006,"range":{"start_line":2006,"start_character":71,"end_line":2006,"end_character":72},"in_reply_to":"3fa7e38b_1b514f64","updated":"2019-12-19 16:35:38.000000000","message":"Done","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"b8676831d671aeb60132c36d3bb3ee01053e6bda","unresolved":false,"context_lines":[{"line_number":2003,"context_line":"    uuids \u003d []"},{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_e467e851","line":2006,"range":{"start_line":2006,"start_character":74,"end_line":2006,"end_character":75},"in_reply_to":"3fa7e38b_3b4c4bc8","updated":"2019-12-19 16:35:38.000000000","message":"Done","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4905a1a04cabc54545575e87eb3d5a80b79a5484","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"},{"line_number":2010,"context_line":"        uuids.append(bdm.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_db4ad7ab","line":2007,"range":{"start_line":2007,"start_character":67,"end_line":2007,"end_character":68},"updated":"2019-12-18 22:49:55.000000000","message":"...to here","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"b8676831d671aeb60132c36d3bb3ee01053e6bda","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"    query \u003d context.session.query(models.BlockDeviceMapping).filter(and_(("},{"line_number":2005,"context_line":"            models.BlockDeviceMapping.boot_index \u003d\u003d 0), ("},{"line_number":2006,"context_line":"            models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)), ("},{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"},{"line_number":2010,"context_line":"        uuids.append(bdm.instance_uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_c46c2c35","line":2007,"range":{"start_line":2007,"start_character":67,"end_line":2007,"end_character":68},"in_reply_to":"3fa7e38b_db4ad7ab","updated":"2019-12-19 16:35:38.000000000","message":"Done","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4905a1a04cabc54545575e87eb3d5a80b79a5484","unresolved":false,"context_lines":[{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"},{"line_number":2010,"context_line":"        uuids.append(bdm.instance_uuid)"},{"line_number":2011,"context_line":""},{"line_number":2012,"context_line":"    return uuids"},{"line_number":2013,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_bbc59b0b","line":2010,"updated":"2019-12-18 22:49:55.000000000","message":"There should be a way to just get the instance_uuid column value out of the query so we don\u0027t have to pull the entire object, so like what you were doing before but without model_query.\n\nWe do something similar here:\n\nhttps://github.com/openstack/nova/blob/a5b8217f5f8013008aec52ad9c9803918631d3c6/nova/objects/cell_mapping.py#L280\n\nSo maybe you can just do:\n\nquery \u003d context.session.query(models.BlockDeviceMapping.instance_uuid)\n...\n\nAnother example is here:\n\nhttps://github.com/openstack/nova/blob/a5b8217f5f8013008aec52ad9c9803918631d3c6/nova/objects/instance.py#L1325","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b7fbb6d1129ccda6d8d187338a32ce8f8e3a0dc3","unresolved":false,"context_lines":[{"line_number":2007,"context_line":"            models.BlockDeviceMapping.destination_type \u003d\u003d \u0027volume\u0027)))"},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":"    for bdm in query.all():"},{"line_number":2010,"context_line":"        uuids.append(bdm.instance_uuid)"},{"line_number":2011,"context_line":""},{"line_number":2012,"context_line":"    return uuids"},{"line_number":2013,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_de715be1","line":2010,"in_reply_to":"3fa7e38b_bbc59b0b","updated":"2019-12-19 15:05:37.000000000","message":"This works for me:\n\nhttp://paste.openstack.org/show/787794/","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"fa1c671f1bfd30a6ca00bea2d238393ebd66105c","unresolved":false,"context_lines":[{"line_number":2000,"context_line":"        return []"},{"line_number":2001,"context_line":""},{"line_number":2002,"context_line":"    result \u003d context.session.query("},{"line_number":2003,"context_line":"        models.BlockDeviceMapping.instance_uuid).filter_by("},{"line_number":2004,"context_line":"        boot_index\u003d0).filter_by("},{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_0f6014d0","line":2003,"updated":"2019-12-20 10:43:01.000000000","message":"I like this code style.","commit_id":"249818b8d8f3a6d0b2e6fa768ae5929b314312ec"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8d46a0379ffceb680236478239054e70ac19ba8b","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res for res, in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_b9f58786","line":2008,"range":{"start_line":2008,"start_character":23,"end_line":2008,"end_character":24},"updated":"2019-12-20 16:53:17.000000000","message":"Why is this here? Looks really odd - is it because result is a list of tuples? If so, this would be clearer:\n\n# result is a list of tuples and we want the first entry\nreturn [res[0] for res in result]","commit_id":"18085684035c8ac8e1a6fe4e79cb093e02a45245"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"a766948d8113ba2048a7b40c622c5cc368e44fe2","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res for res, in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_57642149","line":2008,"range":{"start_line":2008,"start_character":23,"end_line":2008,"end_character":24},"in_reply_to":"3fa7e38b_b9f58786","updated":"2020-01-03 10:02:18.000000000","message":"Done","commit_id":"18085684035c8ac8e1a6fe4e79cb093e02a45245"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1df1b6bc50cfdd9fe0da66fc38d0fbbfdb9006d1","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res[0] for res in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_22d2fe34","line":2008,"updated":"2020-01-14 18:43:17.000000000","message":"This is a pretty wasteful way to determine the state of a boolean. Especially if we\u0027re going to be frequently asking for this column for API filtering. What you have here effectively makes us load the instance list, then go back and query again for a list of booleans.\n\nThere are two things that I think would be a lot better:\n\n1. We could add an actual field to the instance record. Whenever we go to save and instance\u0027s BDMs, we could update that field. When we load existing instances from the database where that field is not currently set, we can calculate it as you\u0027ve done here, but eventually we heal our data set to store that field we want.\n\n2. We could at least alter the the instance query itself to calculate this once per instance at the SQL level. Basically, join BDMs to instances but in a way that gets us one more column with the desired boolean.\n\nEither of these seem a lot better to me than just tacking on another query to this list. I\u0027d like to hear from others how they feel about this.","commit_id":"b289cdb0b3f0f0c6afede0dc1100cb1ab088e84e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"a0ae0fed04c6c04c5ddf32721330aca2f2a9d6bb","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res[0] for res in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_92b8c478","line":2008,"in_reply_to":"3fa7e38b_22d2fe34","updated":"2020-01-16 01:40:27.000000000","message":"\u003e This is a pretty wasteful way to determine the state of a boolean.\n \u003e Especially if we\u0027re going to be frequently asking for this column\n \u003e for API filtering. What you have here effectively makes us load the\n \u003e instance list, then go back and query again for a list of booleans.\n \u003e \n \u003e There are two things that I think would be a lot better:\n \u003e \n \u003e 1. We could add an actual field to the instance record. Whenever we\n \u003e go to save and instance\u0027s BDMs, we could update that field. When we\n \u003e load existing instances from the database where that field is not\n \u003e currently set, we can calculate it as you\u0027ve done here, but\n \u003e eventually we heal our data set to store that field we want.\n \u003e \n\nWe can\u0027t change a BFV instance to normal instance. So we just need to set that field when create the instance, needn\u0027t always sync it. It needs the handle upgrade, as you said heal the existing instance.\n\nThere are multiple calling for this method https://github.com/openstack/nova/blob/24bf2aaa7441430a84b475efca9f4777dc243452/nova/compute/utils.py#L265, those calling can be saved.\n\n \u003e 2. We could at least alter the the instance query itself to\n \u003e calculate this once per instance at the SQL level. Basically, join\n \u003e BDMs to instances but in a way that gets us one more column with\n \u003e the desired boolean.\n \u003e \n\nyea, this should be better than the correct way. This fix is easy than #1, but this can\u0027t save the case https://github.com/openstack/nova/blob/24bf2aaa7441430a84b475efca9f4777dc243452/nova/compute/utils.py#L265. \n\n \u003e Either of these seem a lot better to me than just tacking on\n \u003e another query to this list. I\u0027d like to hear from others how they\n \u003e feel about this.","commit_id":"b289cdb0b3f0f0c6afede0dc1100cb1ab088e84e"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"dd7607c2fb5aa9416acb9e2ad0dc66399b896514","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res[0] for res in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_ca4795a4","line":2008,"in_reply_to":"3fa7e38b_39f8227c","updated":"2020-01-31 01:29:27.000000000","message":"yea, I little prefer the #2, since it is easy than the #1. \n\nBut from the API change, this sounds like need a spec.","commit_id":"b289cdb0b3f0f0c6afede0dc1100cb1ab088e84e"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"539bd3b2388338abb9bf07e711d57a64ae196ffc","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res[0] for res in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_bb4a7b57","line":2008,"in_reply_to":"3fa7e38b_92b8c478","updated":"2020-01-16 07:06:45.000000000","message":"Let\u0027s say we add a new column \u0027volume_backed\u0027 in the `instances` db table in upgrade db script.\n\nWe can also write logic to set appropriate value to the \u0027volume_backed\u0027 column depending on the bdms db records for that instance in the same script so that we don\u0027t have to write separate query to set that fields as it\u0027s done here.\n\nDo you see any issues in setting \u0027volume_backed\u0027 field on the existing instances in the upgrade db script?\n\nFirst seem better solution, I\u0027d like to hear from others on this.","commit_id":"b289cdb0b3f0f0c6afede0dc1100cb1ab088e84e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c180a6ce581d440482b2aa45fa30917fd724e66b","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res[0] for res in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_39f8227c","line":2008,"in_reply_to":"3fa7e38b_bb4a7b57","updated":"2020-01-29 14:59:10.000000000","message":"No, it is not okay to make the db schema upgrade script iterate all instances currently in the database. You\u0027d need to add that field and the migrate it with an online data migration and at load time in the object.\n\nBut I think Alex is saying he\u0027d prefer to _not_ do that and instead calculate the volume-backed nature in SQL via joins (i.e. my #2).\n\nAlex is that correct?","commit_id":"b289cdb0b3f0f0c6afede0dc1100cb1ab088e84e"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"00c59f94190226967ff6f6277e79ddf214ec95e8","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        destination_type\u003d\u0027volume\u0027).filter("},{"line_number":2006,"context_line":"        models.BlockDeviceMapping.instance_uuid.in_(instance_uuids)).all()"},{"line_number":2007,"context_line":""},{"line_number":2008,"context_line":"    return [res[0] for res in result]"},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":""},{"line_number":2011,"context_line":"def _manual_join_columns(columns_to_join):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_dd681d1e","line":2008,"in_reply_to":"3fa7e38b_ca4795a4","updated":"2020-03-05 14:45:53.000000000","message":"Agree, #2 looks better for me.","commit_id":"b289cdb0b3f0f0c6afede0dc1100cb1ab088e84e"}],"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":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":958,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":959,"context_line":"        now \u003d datetime.datetime(2013, 10, 10, 17, 16, 37, 156701)"},{"line_number":960,"context_line":"        instance \u003d self.create_instance_with_args(display_name\u003d\u0027bfv\u0027)"},{"line_number":961,"context_line":"        bdm \u003d {"},{"line_number":962,"context_line":"            \u0027volume_id\u0027: uuidsentinel.uuid1,"},{"line_number":963,"context_line":"            \u0027device_name\u0027: \u0027/dev/vdb\u0027,"},{"line_number":964,"context_line":"            \u0027instance_uuid\u0027: instance[\u0027uuid\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_1d53473c","line":961,"updated":"2019-12-05 14:24:21.000000000","message":"This is really the bare minimum amount of testing for the query code you have. I could think of a few more things to include in the test:\n\n* having a bdm where the boot_index !\u003d 0 (not volume backed)\n* having a bdm where the destination_type \u003d image (not volume backed)\n* having an instance with no bdms\n* playing with deleted instances (so do what you have here but delete the instance which will automatically also delete the bdm) and then query for deleted instances and get that one back to make sure that read_deleted\u003d\u0027yes\u0027 clause is being used in the model query\n* have a query where columns_to_join does not contain \u0027is_volume_backed\u0027 and assert the resulting instance model does not have an \u0027is_volume_backed\u0027 attribute set.","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"16cf41cd5e165600d27b45c37ccf588715122ef8","unresolved":false,"context_lines":[{"line_number":958,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":959,"context_line":"        now \u003d datetime.datetime(2013, 10, 10, 17, 16, 37, 156701)"},{"line_number":960,"context_line":"        instance \u003d self.create_instance_with_args(display_name\u003d\u0027bfv\u0027)"},{"line_number":961,"context_line":"        bdm \u003d {"},{"line_number":962,"context_line":"            \u0027volume_id\u0027: uuidsentinel.uuid1,"},{"line_number":963,"context_line":"            \u0027device_name\u0027: \u0027/dev/vdb\u0027,"},{"line_number":964,"context_line":"            \u0027instance_uuid\u0027: instance[\u0027uuid\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_8d1c9ec2","line":961,"in_reply_to":"3fa7e38b_1d53473c","updated":"2019-12-11 07:22:13.000000000","message":"Done","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2fc29402c8aa50640f166dcf07a8a20c8a354fd","unresolved":false,"context_lines":[{"line_number":966,"context_line":"            \u0027boot_index\u0027: 0"},{"line_number":967,"context_line":"        }"},{"line_number":968,"context_line":"        db.block_device_mapping_create(ctxt, bdm, legacy\u003dFalse)"},{"line_number":969,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":970,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":971,"context_line":"        self.assertTrue(result[0][\u0027is_volume_backed\u0027])"},{"line_number":972,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_7dacbb34","line":969,"range":{"start_line":969,"start_character":32,"end_line":969,"end_character":68},"updated":"2019-12-05 14:24:21.000000000","message":"This seems like kind of a weird method to use for testing this. I mean, it works, and I realize you\u0027re using this because of the os-simple-tenant-usage API at the end of this series, but I probably would have just use instance_get_all.","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"16cf41cd5e165600d27b45c37ccf588715122ef8","unresolved":false,"context_lines":[{"line_number":966,"context_line":"            \u0027boot_index\u0027: 0"},{"line_number":967,"context_line":"        }"},{"line_number":968,"context_line":"        db.block_device_mapping_create(ctxt, bdm, legacy\u003dFalse)"},{"line_number":969,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":970,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":971,"context_line":"        self.assertTrue(result[0][\u0027is_volume_backed\u0027])"},{"line_number":972,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_ad171a9d","line":969,"range":{"start_line":969,"start_character":32,"end_line":969,"end_character":68},"in_reply_to":"3fa7e38b_7dacbb34","updated":"2019-12-11 07:22:13.000000000","message":"Done","commit_id":"4634ce544db077901a564f929abe37b29ca67d93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":965,"context_line":"            \u0027boot_index\u0027: 0"},{"line_number":966,"context_line":"        }"},{"line_number":967,"context_line":"        db.block_device_mapping_create(ctxt, bdm, legacy\u003dFalse)"},{"line_number":968,"context_line":"        self.create_instance_with_args(display_name\u003d\u0027non_bfv\u0027)"},{"line_number":969,"context_line":"        results \u003d sqlalchemy_api.instance_get_all("},{"line_number":970,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":971,"context_line":"        for result in results:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_0751380e","line":968,"updated":"2019-12-13 21:06:02.000000000","message":"OK so this instance doesn\u0027t have any BDMs at all.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":974,"context_line":"            else:"},{"line_number":975,"context_line":"                self.assertFalse(result[\u0027is_volume_backed\u0027])"},{"line_number":976,"context_line":""},{"line_number":977,"context_line":"    def test_instance_get_all_no_joins(self):"},{"line_number":978,"context_line":"        \"\"\"Verify instance model doesn\u0027t have \u0027is_volume_backed\u0027 attribute."},{"line_number":979,"context_line":""},{"line_number":980,"context_line":"        This test will not request \u0027is_volume_backed\u0027 in the columns_to_join"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_276f94c5","line":977,"updated":"2019-12-13 21:06:02.000000000","message":"Could we also mock _get_volume_backed_instances in this test and assert it isn\u0027t called? My point being, let\u0027s make sure we don\u0027t do the unnecessary query if we\u0027re not joining on is_volume_backed. We want to avoid a regression at some point where let\u0027s say the code is refactored and we\u0027re always doing the query but just throwing the results away later because is_volume_backed is not in columns_to_join.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":974,"context_line":"            else:"},{"line_number":975,"context_line":"                self.assertFalse(result[\u0027is_volume_backed\u0027])"},{"line_number":976,"context_line":""},{"line_number":977,"context_line":"    def test_instance_get_all_no_joins(self):"},{"line_number":978,"context_line":"        \"\"\"Verify instance model doesn\u0027t have \u0027is_volume_backed\u0027 attribute."},{"line_number":979,"context_line":""},{"line_number":980,"context_line":"        This test will not request \u0027is_volume_backed\u0027 in the columns_to_join"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_0e2281a4","line":977,"in_reply_to":"3fa7e38b_276f94c5","updated":"2019-12-18 07:06:32.000000000","message":"Done","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":984,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":985,"context_line":"        now \u003d datetime.datetime(2013, 10, 10, 17, 16, 37, 156701)"},{"line_number":986,"context_line":"        self.create_instance_with_args(display_name\u003d\u0027non_bfv\u0027)"},{"line_number":987,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":988,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[])"},{"line_number":989,"context_line":"        self.assertRaises(KeyError, lambda: result[0][\u0027is_volume_backed\u0027])"},{"line_number":990,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c73a40c4","line":987,"range":{"start_line":987,"start_character":32,"end_line":987,"end_character":68},"updated":"2019-12-13 21:06:02.000000000","message":"Again, just use instance_get_all and then we don\u0027t need to mess with the datetime stuff.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":984,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":985,"context_line":"        now \u003d datetime.datetime(2013, 10, 10, 17, 16, 37, 156701)"},{"line_number":986,"context_line":"        self.create_instance_with_args(display_name\u003d\u0027non_bfv\u0027)"},{"line_number":987,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":988,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[])"},{"line_number":989,"context_line":"        self.assertRaises(KeyError, lambda: result[0][\u0027is_volume_backed\u0027])"},{"line_number":990,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_2e23bdac","line":987,"range":{"start_line":987,"start_character":32,"end_line":987,"end_character":68},"in_reply_to":"3fa7e38b_c73a40c4","updated":"2019-12-18 07:06:32.000000000","message":"Done","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":986,"context_line":"        self.create_instance_with_args(display_name\u003d\u0027non_bfv\u0027)"},{"line_number":987,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":988,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[])"},{"line_number":989,"context_line":"        self.assertRaises(KeyError, lambda: result[0][\u0027is_volume_backed\u0027])"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def test_instance_get_active_by_window_joined_bdms_for_deleted_inst(self):"},{"line_number":992,"context_line":"        \"\"\"Verify read_deleted\u003d\u0027yes\u0027 clause being used in the model query."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_a7414439","line":989,"updated":"2019-12-13 21:06:02.000000000","message":"This is kind of gross. There should be a single result so can\u0027t we just do this:\n\nself.assertNotIn(\u0027is_volume_backed\u0027, result[0])\n\n?","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":986,"context_line":"        self.create_instance_with_args(display_name\u003d\u0027non_bfv\u0027)"},{"line_number":987,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":988,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[])"},{"line_number":989,"context_line":"        self.assertRaises(KeyError, lambda: result[0][\u0027is_volume_backed\u0027])"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def test_instance_get_active_by_window_joined_bdms_for_deleted_inst(self):"},{"line_number":992,"context_line":"        \"\"\"Verify read_deleted\u003d\u0027yes\u0027 clause being used in the model query."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_4e287984","line":989,"in_reply_to":"3fa7e38b_a7414439","updated":"2019-12-18 07:06:32.000000000","message":"Done","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":1006,"context_line":"            \u0027boot_index\u0027: 0"},{"line_number":1007,"context_line":"        }"},{"line_number":1008,"context_line":"        db.block_device_mapping_create(ctxt, bdm, legacy\u003dFalse)"},{"line_number":1009,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":1010,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1011,"context_line":"        self.assertTrue(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1012,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c76320de","line":1009,"range":{"start_line":1009,"start_character":32,"end_line":1009,"end_character":68},"updated":"2019-12-13 21:06:02.000000000","message":"same - use instance_get_all, here and below / throughout for the new tests; make sure you rename the test methods as well.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":1006,"context_line":"            \u0027boot_index\u0027: 0"},{"line_number":1007,"context_line":"        }"},{"line_number":1008,"context_line":"        db.block_device_mapping_create(ctxt, bdm, legacy\u003dFalse)"},{"line_number":1009,"context_line":"        result \u003d sqlalchemy_api.instance_get_active_by_window_joined("},{"line_number":1010,"context_line":"            ctxt, begin\u003dnow, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1011,"context_line":"        self.assertTrue(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1012,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_8e143143","line":1009,"range":{"start_line":1009,"start_character":32,"end_line":1009,"end_character":68},"in_reply_to":"3fa7e38b_c76320de","updated":"2019-12-18 07:06:32.000000000","message":"done, changed to instance_get_all","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"        self.assertFalse(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1038,"context_line":""},{"line_number":1039,"context_line":"    def test_instance_get_all_bdms_boot_from_image(self):"},{"line_number":1040,"context_line":"        \"\"\"Verify instance model for instance booted from image dbms."},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"        This test will request \u0027is_volume_backed\u0027 in the columns_to_join for"},{"line_number":1043,"context_line":"        instances booted from image and verify that the instance model has"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_679a6cc5","line":1040,"range":{"start_line":1040,"start_character":64,"end_line":1040,"end_character":68},"updated":"2019-12-13 21:06:02.000000000","message":"bdms","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"        self.assertFalse(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1038,"context_line":""},{"line_number":1039,"context_line":"    def test_instance_get_all_bdms_boot_from_image(self):"},{"line_number":1040,"context_line":"        \"\"\"Verify instance model for instance booted from image dbms."},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"        This test will request \u0027is_volume_backed\u0027 in the columns_to_join for"},{"line_number":1043,"context_line":"        instances booted from image and verify that the instance model has"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_6e177548","line":1040,"range":{"start_line":1040,"start_character":64,"end_line":1040,"end_character":68},"in_reply_to":"3fa7e38b_679a6cc5","updated":"2019-12-18 07:06:32.000000000","message":"corrected","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a8bda0a7771601eafb6957130ee76710948eea9e","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1059,"context_line":"        self.assertFalse(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    def test_instance_get_all_with_no_bdms(self):"},{"line_number":1062,"context_line":"        \"\"\"Verify instance model for instance booted from image."},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"        This test will request \u0027is_volume_backed\u0027 in the columns_to_join for"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_87b2084a","line":1061,"range":{"start_line":1061,"start_character":8,"end_line":1061,"end_character":42},"updated":"2019-12-13 21:06:02.000000000","message":"This is redundant with the non_bfv instance created in test_instance_get_all_with_instances_bfv above. So either remove this duplicate test or remove the no-bdms instance from that other test.","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"ad58850d2f29953af68c865b7bcf629268863b86","unresolved":false,"context_lines":[{"line_number":1058,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1059,"context_line":"        self.assertFalse(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    def test_instance_get_all_with_no_bdms(self):"},{"line_number":1062,"context_line":"        \"\"\"Verify instance model for instance booted from image."},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"        This test will request \u0027is_volume_backed\u0027 in the columns_to_join for"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_ce0a291d","line":1061,"range":{"start_line":1061,"start_character":8,"end_line":1061,"end_character":42},"in_reply_to":"3fa7e38b_87b2084a","updated":"2019-12-18 07:06:32.000000000","message":"removed","commit_id":"6f9c13763a108cea8822e54ef9a7bb4fb8d30bea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4905a1a04cabc54545575e87eb3d5a80b79a5484","unresolved":false,"context_lines":[{"line_number":990,"context_line":""},{"line_number":991,"context_line":"        self.assertNotIn(\u0027is_volume_backed\u0027, result[0])"},{"line_number":992,"context_line":"        mock_get_volume_backed_instances.assert_not_called()"},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"    def test_instance_get_all_with_bdms_as_secondary_volume(self):"},{"line_number":995,"context_line":"        \"\"\"Verify instance model for instance with boot_index !\u003d 0"},{"line_number":996,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_dbdf37d5","line":993,"updated":"2019-12-18 22:49:55.000000000","message":"You removed test_instance_get_active_by_window_joined_bdms_for_deleted_inst since PS2 so you lost coverage of the deleted case, why? I had commented to just change that to use instance_get_all rather than instance_get_active_by_window_joined but you removed the entire test.","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"b8676831d671aeb60132c36d3bb3ee01053e6bda","unresolved":false,"context_lines":[{"line_number":990,"context_line":""},{"line_number":991,"context_line":"        self.assertNotIn(\u0027is_volume_backed\u0027, result[0])"},{"line_number":992,"context_line":"        mock_get_volume_backed_instances.assert_not_called()"},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"    def test_instance_get_all_with_bdms_as_secondary_volume(self):"},{"line_number":995,"context_line":"        \"\"\"Verify instance model for instance with boot_index !\u003d 0"},{"line_number":996,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_2457405d","line":993,"in_reply_to":"3fa7e38b_dbdf37d5","updated":"2019-12-19 16:35:38.000000000","message":"added as suggested.","commit_id":"ddf7ba3d361eb78a7026287ae7e2b45a1e0cf01e"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"fa1c671f1bfd30a6ca00bea2d238393ebd66105c","unresolved":false,"context_lines":[{"line_number":979,"context_line":"        self.create_instance_with_args(display_name\u003d\u0027non_bfv\u0027)"},{"line_number":980,"context_line":"        results \u003d sqlalchemy_api.instance_get_all("},{"line_number":981,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":982,"context_line":"        for result in results:"},{"line_number":983,"context_line":"            if result[\u0027display_name\u0027] \u003d\u003d \u0027bfv\u0027:"},{"line_number":984,"context_line":"                self.assertTrue(result[\u0027is_volume_backed\u0027])"},{"line_number":985,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_ef8fb82c","line":982,"updated":"2019-12-20 10:43:01.000000000","message":"nit: I think here if assert number of the result will be better.\n\nsuch as: self.assertEqual(3, len(results))","commit_id":"249818b8d8f3a6d0b2e6fa768ae5929b314312ec"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"003629306e49546db29f69b6b83bdacb026259bb","unresolved":false,"context_lines":[{"line_number":979,"context_line":"        self.create_instance_with_args(display_name\u003d\u0027non_bfv\u0027)"},{"line_number":980,"context_line":"        results \u003d sqlalchemy_api.instance_get_all("},{"line_number":981,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":982,"context_line":"        for result in results:"},{"line_number":983,"context_line":"            if result[\u0027display_name\u0027] \u003d\u003d \u0027bfv\u0027:"},{"line_number":984,"context_line":"                self.assertTrue(result[\u0027is_volume_backed\u0027])"},{"line_number":985,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_ea1c8615","line":982,"in_reply_to":"3fa7e38b_ef8fb82c","updated":"2019-12-20 11:48:16.000000000","message":"Done","commit_id":"249818b8d8f3a6d0b2e6fa768ae5929b314312ec"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"fa1c671f1bfd30a6ca00bea2d238393ebd66105c","unresolved":false,"context_lines":[{"line_number":1021,"context_line":"        db.block_device_mapping_create(ctxt, bdm, legacy\u003dFalse)"},{"line_number":1022,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1023,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1024,"context_line":"        self.assertTrue(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1025,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"},{"line_number":1026,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1027,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_cf6d1c68","line":1024,"updated":"2019-12-20 10:43:01.000000000","message":"nit: Beacause of you will assert the number of the instance after destroy, if first to assert the number, it will be more persuasive. e.g. self.assertEqual(1, len(result))","commit_id":"249818b8d8f3a6d0b2e6fa768ae5929b314312ec"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"003629306e49546db29f69b6b83bdacb026259bb","unresolved":false,"context_lines":[{"line_number":1021,"context_line":"        db.block_device_mapping_create(ctxt, bdm, legacy\u003dFalse)"},{"line_number":1022,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1023,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1024,"context_line":"        self.assertTrue(result[0][\u0027is_volume_backed\u0027])"},{"line_number":1025,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"},{"line_number":1026,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1027,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_0a18021d","line":1024,"in_reply_to":"3fa7e38b_cf6d1c68","updated":"2019-12-20 11:48:16.000000000","message":"Done","commit_id":"249818b8d8f3a6d0b2e6fa768ae5929b314312ec"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8d46a0379ffceb680236478239054e70ac19ba8b","unresolved":false,"context_lines":[{"line_number":1027,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"},{"line_number":1028,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1029,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1030,"context_line":"        self.assertEqual(0, len(result))"},{"line_number":1031,"context_line":""},{"line_number":1032,"context_line":"    def test_instance_get_all_with_bdms_as_secondary_volume(self):"},{"line_number":1033,"context_line":"        \"\"\"Verify instance model for instance with boot_index !\u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_7944efb8","line":1030,"range":{"start_line":1030,"start_character":8,"end_line":1030,"end_character":40},"updated":"2019-12-20 16:53:17.000000000","message":"Umm, isn\u0027t this *not* what you\u0027d expect? The test description is asserting that you *should* get the deleted instance, right? See how this worked in PS2:\n\nhttps://review.opendev.org/#/c/694462/2/nova/tests/unit/db/test_db_api.py@1013\n\nI\u0027m assuming the issue has something to do with how we\u0027re not using model_query with read_deleted\u003d\"yes\" to get the BDMs anymore. But model_query with read_deleted\u003d\"yes\" just omits the filter on the deleted column:\n\nhttps://github.com/openstack/nova/blob/02019d2660dfce3facdd64ecdb2bd60ba4a91c6d/nova/db/sqlalchemy/api.py#L285\n\nSo the query to get BDMs should return both deleted and non-deleted records, so I\u0027m not sure what\u0027s going on here. You\u0027ll have to debug.","commit_id":"18085684035c8ac8e1a6fe4e79cb093e02a45245"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"a766948d8113ba2048a7b40c622c5cc368e44fe2","unresolved":false,"context_lines":[{"line_number":1027,"context_line":"        db.instance_destroy(ctxt, instance[\u0027uuid\u0027])"},{"line_number":1028,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1029,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1030,"context_line":"        self.assertEqual(0, len(result))"},{"line_number":1031,"context_line":""},{"line_number":1032,"context_line":"    def test_instance_get_all_with_bdms_as_secondary_volume(self):"},{"line_number":1033,"context_line":"        \"\"\"Verify instance model for instance with boot_index !\u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_f7e2ed9b","line":1030,"range":{"start_line":1030,"start_character":8,"end_line":1030,"end_character":40},"in_reply_to":"3fa7e38b_7944efb8","updated":"2020-01-03 10:02:18.000000000","message":"get BDMs method retrieves data based on instance_uuids supplied by instance_get_all()\ninstance_get_all() not considering [1] read_deleted flag, so its None here and read it from context [2] which is \u0027no\u0027 by default [3]\n\n[1]: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2024\n[2]: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L277\n[3]: https://github.com/openstack/nova/blob/master/nova/context.py#L89","commit_id":"18085684035c8ac8e1a6fe4e79cb093e02a45245"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"95c7cffaf40f26247a505e512e116375b3b0f1ae","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"        \"\"\"Verify instance model for deleted instance."},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"        This test will verify that bdms of deleted instances can not be"},{"line_number":1010,"context_line":"        retrieved, as instance_get_all() not considering deleted instances."},{"line_number":1011,"context_line":"        \"\"\""},{"line_number":1012,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":1013,"context_line":"        instance \u003d self.create_instance_with_args(display_name\u003d\u0027bfv\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_bed20dfd","line":1010,"range":{"start_line":1010,"start_character":19,"end_line":1010,"end_character":75},"updated":"2020-01-03 16:45:15.000000000","message":"Not by default no. But you can modify the context passed to that method to use read_deleted\u003d\u0027yes\u0027 and then we should get back deleted instances with is_volume_backed set, correct? Isn\u0027t that something the simple tenant usage API is going to need since it reads usage based on a time window including deleted instances?","commit_id":"304208a22af1c4bb3a6037eed908fda67542e65b"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"515de141b58dc7364b986e0b34a33f4e93f405d8","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"        \"\"\"Verify instance model for deleted instance."},{"line_number":1008,"context_line":""},{"line_number":1009,"context_line":"        This test will verify that bdms of deleted instances can not be"},{"line_number":1010,"context_line":"        retrieved, as instance_get_all() not considering deleted instances."},{"line_number":1011,"context_line":"        \"\"\""},{"line_number":1012,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":1013,"context_line":"        instance \u003d self.create_instance_with_args(display_name\u003d\u0027bfv\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_4e6d60c9","line":1010,"range":{"start_line":1010,"start_character":19,"end_line":1010,"end_character":75},"in_reply_to":"3fa7e38b_bed20dfd","updated":"2020-01-06 11:59:03.000000000","message":"done","commit_id":"304208a22af1c4bb3a6037eed908fda67542e65b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"77c30bb03b91277049daea75a2cc0288fda5c67b","unresolved":false,"context_lines":[{"line_number":1028,"context_line":"        ctxt \u003d context.get_admin_context(read_deleted\u003d\u0027yes\u0027)"},{"line_number":1029,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1030,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1031,"context_line":"        self.assertEqual(1, len(result))"},{"line_number":1032,"context_line":""},{"line_number":1033,"context_line":"    def test_instance_get_all_with_bdms_as_secondary_volume(self):"},{"line_number":1034,"context_line":"        \"\"\"Verify instance model for instance with boot_index !\u003d 0"}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_990b997a","line":1031,"updated":"2020-01-06 20:59:29.000000000","message":"This would be good as well:\n\nself.assertTrue(result[0][\u0027is_volume_backed\u0027])","commit_id":"be4f5b1b0ed82097cabe487b1e4c5037a8605996"},{"author":{"_account_id":27302,"name":"Shilpa Devharakar","email":"shilpa.devharakar@nttdata.com","username":"shilpa.devharakar"},"change_message_id":"48bcc5ddf8a3c6d361c3acac441197c88ae5d8f4","unresolved":false,"context_lines":[{"line_number":1028,"context_line":"        ctxt \u003d context.get_admin_context(read_deleted\u003d\u0027yes\u0027)"},{"line_number":1029,"context_line":"        result \u003d sqlalchemy_api.instance_get_all("},{"line_number":1030,"context_line":"            ctxt, columns_to_join\u003d[\u0027is_volume_backed\u0027])"},{"line_number":1031,"context_line":"        self.assertEqual(1, len(result))"},{"line_number":1032,"context_line":""},{"line_number":1033,"context_line":"    def test_instance_get_all_with_bdms_as_secondary_volume(self):"},{"line_number":1034,"context_line":"        \"\"\"Verify instance model for instance with boot_index !\u003d 0"}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_255cb34a","line":1031,"in_reply_to":"3fa7e38b_990b997a","updated":"2020-01-07 10:47:28.000000000","message":"Done","commit_id":"be4f5b1b0ed82097cabe487b1e4c5037a8605996"}]}