)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"3596d84a4f7fae17660b12f7c589048541c24a55","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Note that this now does metadata and system_metadata. I think that since the"},{"line_number":10,"context_line":"schema of the two tables is the same, I may be able to do a union query and"},{"line_number":11,"context_line":"pick them both up in a single go, but I don\u0027t think I\u0027ll be able to do that with"},{"line_number":12,"context_line":"the ORM, but rather have to fall back to regular SQLAlchemy table/column stuff."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Related to bug 1164737"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAOn%2F%2F9b8%3D","line":12,"updated":"2013-04-10 04:16:12.000000000","message":"The speculative stuff here probably belongs in a review comment not in the commit history.","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a332f72617aacc2788372d474009d84a9d236683","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Note that this now does metadata and system_metadata. I think that since the"},{"line_number":10,"context_line":"schema of the two tables is the same, I may be able to do a union query and"},{"line_number":11,"context_line":"pick them both up in a single go, but I don\u0027t think I\u0027ll be able to do that with"},{"line_number":12,"context_line":"the ORM, but rather have to fall back to regular SQLAlchemy table/column stuff."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Related to bug 1164737"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAOn%2F%2F894%3D","line":12,"in_reply_to":"AAAAOn%2F%2F9b8%3D","updated":"2013-04-10 13:37:58.000000000","message":"This has been an evolving state-of-the-union since this whole set has been a work in progress with a few people. I agree it should come out when we\u0027re happy with the rest of the stuff.","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"1996a5527c5187461fbddd7d218d8e223904f783","unresolved":false,"context_lines":[{"line_number":1584,"context_line":"        else:"},{"line_number":1585,"context_line":"            query \u003d query.filter_by(user_id\u003dcontext.user_id)"},{"line_number":1586,"context_line":"    instances \u003d query.all()"},{"line_number":1587,"context_line":"    if \u0027system_metadata\u0027 in columns_to_join:"},{"line_number":1588,"context_line":"        return _instances_fill_system_metadata(context, instances)"},{"line_number":1589,"context_line":"    else:"},{"line_number":1590,"context_line":"        return instances"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAOX%2F%2F8Pg%3D","line":1587,"updated":"2013-04-05 00:46:02.000000000","message":"This won\u0027t work alone since the None case above doesn\u0027t have system_metadata in it anymore.","commit_id":"b56af947121527eef1090b496849d96c5721d6c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea02ce1d24863be8e8b1c3a1fd8ef003c1e67afd","unresolved":false,"context_lines":[{"line_number":1584,"context_line":"        else:"},{"line_number":1585,"context_line":"            query \u003d query.filter_by(user_id\u003dcontext.user_id)"},{"line_number":1586,"context_line":"    instances \u003d query.all()"},{"line_number":1587,"context_line":"    if \u0027system_metadata\u0027 in columns_to_join:"},{"line_number":1588,"context_line":"        return _instances_fill_system_metadata(context, instances)"},{"line_number":1589,"context_line":"    else:"},{"line_number":1590,"context_line":"        return instances"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAOX%2F%2F8N0%3D","line":1587,"in_reply_to":"AAAAOX%2F%2F8Pg%3D","updated":"2013-04-05 01:22:35.000000000","message":"Hmm, right. It\u0027s more than a little disturbing that Jenkins thinks its okay, because I would expect we\u0027re filtering out system_metadata in a lot of places then. I\u0027ll look closer.","commit_id":"b56af947121527eef1090b496849d96c5721d6c3"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"f1ab62b054be0dc9ba9ffb9e83c328eb1d4313fb","unresolved":false,"context_lines":[{"line_number":1793,"context_line":""},{"line_number":1794,"context_line":"    return _instances_fill_system_metadata(context,"},{"line_number":1795,"context_line":"        model_query(context, models.Instance)."},{"line_number":1796,"context_line":"            options(joinedload(\u0027system_metadata\u0027))."},{"line_number":1797,"context_line":"            filter(models.Instance.updated_at \u003c\u003d reboot_window)."},{"line_number":1798,"context_line":"            filter_by(task_state\u003dtask_states.REBOOTING).all())"},{"line_number":1799,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOX%2F%2F7W8%3D","line":1796,"updated":"2013-04-05 17:11:16.000000000","message":"Forgot to remove this join.","commit_id":"05cdb1736cde4cd31dda232fa2481032af4f120e"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"ef59c91f2a0f1938a0c0ff21c5e0e823205c55db","unresolved":false,"context_lines":[{"line_number":1554,"context_line":"            options(joinedload(\u0027system_metadata\u0027))"},{"line_number":1555,"context_line":""},{"line_number":1556,"context_line":""},{"line_number":1557,"context_line":"def __instances_fill_metadata(context, instances, do_metadata, do_sys_meta):"},{"line_number":1558,"context_line":"    uuids \u003d [inst[\u0027uuid\u0027] for inst in instances]"},{"line_number":1559,"context_line":"    if do_metadata:"},{"line_number":1560,"context_line":"        meta_rows \u003d _instance_metadata_get_multi(context, uuids)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2BmU%3D","line":1557,"updated":"2013-04-09 13:20:38.000000000","message":"So basically you\u0027re doing all this to bulk load metadata separately without breaking backward compatibility?\n\nI hope the idea is to do this now and then reduce the uses of the method that expect to get \"joined\" (sys)metadata.","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3affb93576ce881a5ad623e324eca6c054233123","unresolved":false,"context_lines":[{"line_number":1554,"context_line":"            options(joinedload(\u0027system_metadata\u0027))"},{"line_number":1555,"context_line":""},{"line_number":1556,"context_line":""},{"line_number":1557,"context_line":"def __instances_fill_metadata(context, instances, do_metadata, do_sys_meta):"},{"line_number":1558,"context_line":"    uuids \u003d [inst[\u0027uuid\u0027] for inst in instances]"},{"line_number":1559,"context_line":"    if do_metadata:"},{"line_number":1560,"context_line":"        meta_rows \u003d _instance_metadata_get_multi(context, uuids)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2Bj0%3D","line":1557,"in_reply_to":"AAAAOn%2F%2F%2BmU%3D","updated":"2013-04-09 13:38:36.000000000","message":"Yep, see the later patches in this set.","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"ef59c91f2a0f1938a0c0ff21c5e0e823205c55db","unresolved":false,"context_lines":[{"line_number":1560,"context_line":"        meta_rows \u003d _instance_metadata_get_multi(context, uuids)"},{"line_number":1561,"context_line":"    else:"},{"line_number":1562,"context_line":"        meta_rows \u003d []"},{"line_number":1563,"context_line":"    meta \u003d collections.defaultdict(list)"},{"line_number":1564,"context_line":"    for row in meta_rows:"},{"line_number":1565,"context_line":"        meta[row[\u0027instance_uuid\u0027]].append(row)"},{"line_number":1566,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2BmI%3D","line":1563,"updated":"2013-04-09 13:20:38.000000000","message":"Interesting, never seen this before :)","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"ef59c91f2a0f1938a0c0ff21c5e0e823205c55db","unresolved":false,"context_lines":[{"line_number":1587,"context_line":""},{"line_number":1588,"context_line":""},{"line_number":1589,"context_line":"@require_context"},{"line_number":1590,"context_line":"def instance_get_all(context, columns_to_join\u003dNone):"},{"line_number":1591,"context_line":"    if columns_to_join is None:"},{"line_number":1592,"context_line":"        do_sys_meta \u003d True"},{"line_number":1593,"context_line":"        columns_to_join \u003d [\u0027info_cache\u0027, \u0027security_groups\u0027, \u0027metadata\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2Bmk%3D","line":1590,"updated":"2013-04-09 13:20:38.000000000","message":"if columns_to_join \u003d\u003d [\u0027system_metadata\u0027], then both metadata and system_metadata get loaded and \"filled in\"?","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3affb93576ce881a5ad623e324eca6c054233123","unresolved":false,"context_lines":[{"line_number":1587,"context_line":""},{"line_number":1588,"context_line":""},{"line_number":1589,"context_line":"@require_context"},{"line_number":1590,"context_line":"def instance_get_all(context, columns_to_join\u003dNone):"},{"line_number":1591,"context_line":"    if columns_to_join is None:"},{"line_number":1592,"context_line":"        do_sys_meta \u003d True"},{"line_number":1593,"context_line":"        columns_to_join \u003d [\u0027info_cache\u0027, \u0027security_groups\u0027, \u0027metadata\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2Bj4%3D","line":1590,"in_reply_to":"AAAAOn%2F%2F%2Bmk%3D","updated":"2013-04-09 13:38:36.000000000","message":"Oops, yeah, need to fix this up since I added metadata. Since nothing uses this function though.. :D","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3affb93576ce881a5ad623e324eca6c054233123","unresolved":false,"context_lines":[{"line_number":1818,"context_line":"def _instance_get_all_query(context, project_only\u003dFalse, joins\u003dNone):"},{"line_number":1819,"context_line":"    if joins is None:"},{"line_number":1820,"context_line":"        joins \u003d [\u0027info_cache\u0027, \u0027security_groups\u0027, \u0027metadata\u0027,"},{"line_number":1821,"context_line":"                 \u0027system_metadata\u0027]"},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"    query \u003d model_query(context, models.Instance, project_only\u003dproject_only)"},{"line_number":1824,"context_line":"    for join in joins:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2Bjs%3D","line":1821,"updated":"2013-04-09 13:38:36.000000000","message":"Note that I think I missed removing these joins.","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a225485775dfa138594a3cdce8eea61b3f2ed085","unresolved":false,"context_lines":[{"line_number":1818,"context_line":"def _instance_get_all_query(context, project_only\u003dFalse, joins\u003dNone):"},{"line_number":1819,"context_line":"    if joins is None:"},{"line_number":1820,"context_line":"        joins \u003d [\u0027info_cache\u0027, \u0027security_groups\u0027, \u0027metadata\u0027,"},{"line_number":1821,"context_line":"                 \u0027system_metadata\u0027]"},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"    query \u003d model_query(context, models.Instance, project_only\u003dproject_only)"},{"line_number":1824,"context_line":"    for join in joins:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2BXs%3D","line":1821,"in_reply_to":"AAAAOn%2F%2F%2BeY%3D","updated":"2013-04-09 15:18:27.000000000","message":"Yeah, although I wonder if it wouldn\u0027t be better (long term) to remove the instance_get_all() API anyway, to further discourage new code from fetching the entire state of the universe :)","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"394158098e45c1b791301c627d2cfe772e671df4","unresolved":false,"context_lines":[{"line_number":1818,"context_line":"def _instance_get_all_query(context, project_only\u003dFalse, joins\u003dNone):"},{"line_number":1819,"context_line":"    if joins is None:"},{"line_number":1820,"context_line":"        joins \u003d [\u0027info_cache\u0027, \u0027security_groups\u0027, \u0027metadata\u0027,"},{"line_number":1821,"context_line":"                 \u0027system_metadata\u0027]"},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"    query \u003d model_query(context, models.Instance, project_only\u003dproject_only)"},{"line_number":1824,"context_line":"    for join in joins:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2BeY%3D","line":1821,"in_reply_to":"AAAAOn%2F%2F%2Bjs%3D","updated":"2013-04-09 14:38:01.000000000","message":"You could refactor instance_get_all() to use this method and eliminate some cut and paste.\n\n(One returns instances, and the other returns a query set)","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"ef59c91f2a0f1938a0c0ff21c5e0e823205c55db","unresolved":false,"context_lines":[{"line_number":3832,"context_line":"########################"},{"line_number":3833,"context_line":"# User-provided metadata"},{"line_number":3834,"context_line":""},{"line_number":3835,"context_line":"def _instance_metadata_get_multi(context, instance_uuids, session\u003dNone):"},{"line_number":3836,"context_line":"    return model_query(context, models.InstanceMetadata,"},{"line_number":3837,"context_line":"                       session\u003dsession).\\"},{"line_number":3838,"context_line":"                    filter("}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2Bls%3D","line":3835,"updated":"2013-04-09 13:20:38.000000000","message":"Basically the same method as below.","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3affb93576ce881a5ad623e324eca6c054233123","unresolved":false,"context_lines":[{"line_number":3832,"context_line":"########################"},{"line_number":3833,"context_line":"# User-provided metadata"},{"line_number":3834,"context_line":""},{"line_number":3835,"context_line":"def _instance_metadata_get_multi(context, instance_uuids, session\u003dNone):"},{"line_number":3836,"context_line":"    return model_query(context, models.InstanceMetadata,"},{"line_number":3837,"context_line":"                       session\u003dsession).\\"},{"line_number":3838,"context_line":"                    filter("}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAOn%2F%2F%2Bjc%3D","line":3835,"in_reply_to":"AAAAOn%2F%2F%2Bls%3D","updated":"2013-04-09 13:38:36.000000000","message":"Except one takes a set operator and one takes a single uuid. I\u0027m sure that checking membership in a set is slower than an equality operation, right? I\u0027d hate to penalize all the instance_get operations as a result...","commit_id":"e2068c11a80b202b6c6f1737cc02b6990254a588"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"3137b12521d25ae20e840782ed0d97b1d6f90d63","unresolved":false,"context_lines":[{"line_number":1557,"context_line":"def __instances_fill_metadata(context, instances, do_metadata, do_sys_meta):"},{"line_number":1558,"context_line":"    uuids \u003d [inst[\u0027uuid\u0027] for inst in instances]"},{"line_number":1559,"context_line":"    if do_metadata:"},{"line_number":1560,"context_line":"        meta_rows \u003d _instance_metadata_get_multi(context, uuids)"},{"line_number":1561,"context_line":"    else:"},{"line_number":1562,"context_line":"        meta_rows \u003d []"},{"line_number":1563,"context_line":"    meta \u003d collections.defaultdict(list)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2F%2BA4%3D","line":1560,"updated":"2013-04-09 18:58:04.000000000","message":"You could combine the get_multi() methods by allowing the model to use to be passed in, I think.  But, I don\u0027t know if that really cleans this up a lot.  Just a random thought as I look at this.","commit_id":"a91724b5eb67d6a2636a894fe630a5f5f24f8ea0"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"d5d1ede9390e67eb9384e20430ab84f7d38331ea","unresolved":false,"context_lines":[{"line_number":1682,"context_line":"                           marker\u003dmarker,"},{"line_number":1683,"context_line":"                           sort_dir\u003dsort_dir)"},{"line_number":1684,"context_line":""},{"line_number":1685,"context_line":"    return _instances_fill_metadata(context, query_prefix.all(),"},{"line_number":1686,"context_line":"                           do_metadata\u003dfilters.get(\u0027do_metadata\u0027, True),"},{"line_number":1687,"context_line":"                           do_sys_meta\u003dfilters.get(\u0027do_sys_meta\u0027, True))"},{"line_number":1688,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2F97o%3D","line":1685,"updated":"2013-04-09 20:05:49.000000000","message":"This almost wants an InstanceFactory class to hide the ugly. :)","commit_id":"a91724b5eb67d6a2636a894fe630a5f5f24f8ea0"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"3137b12521d25ae20e840782ed0d97b1d6f90d63","unresolved":false,"context_lines":[{"line_number":1684,"context_line":""},{"line_number":1685,"context_line":"    return _instances_fill_metadata(context, query_prefix.all(),"},{"line_number":1686,"context_line":"                           do_metadata\u003dfilters.get(\u0027do_metadata\u0027, True),"},{"line_number":1687,"context_line":"                           do_sys_meta\u003dfilters.get(\u0027do_sys_meta\u0027, True))"},{"line_number":1688,"context_line":""},{"line_number":1689,"context_line":""},{"line_number":1690,"context_line":"def tag_filter(query, model, tag_model, tag_model_col, filters):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2F%2BAs%3D","line":1687,"updated":"2013-04-09 18:58:04.000000000","message":"Prob should doc-string this above.","commit_id":"a91724b5eb67d6a2636a894fe630a5f5f24f8ea0"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"3596d84a4f7fae17660b12f7c589048541c24a55","unresolved":false,"context_lines":[{"line_number":1556,"context_line":""},{"line_number":1557,"context_line":"def __instances_fill_metadata(context, instances, manual_joins):"},{"line_number":1558,"context_line":"    \"\"\"Selectively fill instances with manually-joined metadata. Note that"},{"line_number":1559,"context_line":"    instance will be converted to a dict."},{"line_number":1560,"context_line":"    :param context: security context"},{"line_number":1561,"context_line":"    :param instances: list of instances to fill"},{"line_number":1562,"context_line":"    :param manual_joins: list of tables to manually join (can be any"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2F9b4%3D","line":1559,"updated":"2013-04-10 04:16:12.000000000","message":"Needs a blank line between the first line of a docstring and the rest.\nAlso the first line should be a single self contained sentence. http://www.python.org/dev/peps/pep-0257/","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"3596d84a4f7fae17660b12f7c589048541c24a55","unresolved":false,"context_lines":[{"line_number":1575,"context_line":"            sys_meta[row[\u0027instance_uuid\u0027]].append(row)"},{"line_number":1576,"context_line":""},{"line_number":1577,"context_line":"    for inst in instances:"},{"line_number":1578,"context_line":"        inst \u003d dict(inst.iteritems())"},{"line_number":1579,"context_line":"        inst[\u0027system_metadata\u0027] \u003d sys_meta[inst[\u0027uuid\u0027]]"},{"line_number":1580,"context_line":"        inst[\u0027metadata\u0027] \u003d meta[inst[\u0027uuid\u0027]]"},{"line_number":1581,"context_line":"        yield inst"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2F9bU%3D","line":1578,"updated":"2013-04-10 04:16:12.000000000","message":"If inst is a dict (which it sure looks like for iteritems()) to work) you can just say dict(inst)\n\nBut I don\u0027t understand why you are duplicating the instance anyway?\nwhy not just\n\nfor inst in instances:\n    inst[\u0027system_metadata\u0027] \u003d sys_meta[inst[\u0027uuid\u0027]]\n    inst[\u0027metada\u0027] \u003d meta[inst[\u0027uuid\u0027]]\n    yield inst","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"fc731545d23995a124bc8ade61b71aa5c673164c","unresolved":false,"context_lines":[{"line_number":1575,"context_line":"            sys_meta[row[\u0027instance_uuid\u0027]].append(row)"},{"line_number":1576,"context_line":""},{"line_number":1577,"context_line":"    for inst in instances:"},{"line_number":1578,"context_line":"        inst \u003d dict(inst.iteritems())"},{"line_number":1579,"context_line":"        inst[\u0027system_metadata\u0027] \u003d sys_meta[inst[\u0027uuid\u0027]]"},{"line_number":1580,"context_line":"        inst[\u0027metadata\u0027] \u003d meta[inst[\u0027uuid\u0027]]"},{"line_number":1581,"context_line":"        yield inst"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2F8%2Fg%3D","line":1578,"in_reply_to":"AAAAOn%2F%2F9bU%3D","updated":"2013-04-10 13:25:59.000000000","message":"The iteritems is a way to dict-ify the values within the top level dict.  dict(inst) won\u0027t do that.","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"f73299a1af8fa9fdc3599b3c5c5ba8e15c6111d6","unresolved":false,"context_lines":[{"line_number":1575,"context_line":"            sys_meta[row[\u0027instance_uuid\u0027]].append(row)"},{"line_number":1576,"context_line":""},{"line_number":1577,"context_line":"    for inst in instances:"},{"line_number":1578,"context_line":"        inst \u003d dict(inst.iteritems())"},{"line_number":1579,"context_line":"        inst[\u0027system_metadata\u0027] \u003d sys_meta[inst[\u0027uuid\u0027]]"},{"line_number":1580,"context_line":"        inst[\u0027metadata\u0027] \u003d meta[inst[\u0027uuid\u0027]]"},{"line_number":1581,"context_line":"        yield inst"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2F9UU%3D","line":1578,"in_reply_to":"AAAAOn%2F%2F9bU%3D","updated":"2013-04-10 07:02:33.000000000","message":"because it\u0027s a sqlalchemy model.  dict() does not work on them... but they do have iteritems().  And just assigning \u0027system_metadata\u0027 in the model will not work as its a relationship defined in models.py","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"3596d84a4f7fae17660b12f7c589048541c24a55","unresolved":false,"context_lines":[{"line_number":1581,"context_line":"        yield inst"},{"line_number":1582,"context_line":""},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":"def _instances_fill_metadata(context, instances, manual_joins\u003dNone):"},{"line_number":1585,"context_line":"    if manual_joins is None:"},{"line_number":1586,"context_line":"        manual_joins \u003d [\u0027metadata\u0027, \u0027system_metadata\u0027]"},{"line_number":1587,"context_line":"    return list(__instances_fill_metadata(context, instances, manual_joins))"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2F9bQ%3D","line":1584,"updated":"2013-04-10 04:16:12.000000000","message":"I don\u0027t think this warrants being a separate method; I\u0027d just drop one _ from __instances_fill_metadata and check for manual_joins at the top.","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a332f72617aacc2788372d474009d84a9d236683","unresolved":false,"context_lines":[{"line_number":1581,"context_line":"        yield inst"},{"line_number":1582,"context_line":""},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":"def _instances_fill_metadata(context, instances, manual_joins\u003dNone):"},{"line_number":1585,"context_line":"    if manual_joins is None:"},{"line_number":1586,"context_line":"        manual_joins \u003d [\u0027metadata\u0027, \u0027system_metadata\u0027]"},{"line_number":1587,"context_line":"    return list(__instances_fill_metadata(context, instances, manual_joins))"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2F89w%3D","line":1584,"in_reply_to":"AAAAOn%2F%2F9UQ%3D","updated":"2013-04-10 13:37:58.000000000","message":"I did it this way to avoid processing the instances list in its entirety several times. The hope was that as I did the rest of the optimizations, I would find some of the functions that were suitable for returning a generator, but that didn\u0027t happen. So, yeah.","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"f73299a1af8fa9fdc3599b3c5c5ba8e15c6111d6","unresolved":false,"context_lines":[{"line_number":1581,"context_line":"        yield inst"},{"line_number":1582,"context_line":""},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":"def _instances_fill_metadata(context, instances, manual_joins\u003dNone):"},{"line_number":1585,"context_line":"    if manual_joins is None:"},{"line_number":1586,"context_line":"        manual_joins \u003d [\u0027metadata\u0027, \u0027system_metadata\u0027]"},{"line_number":1587,"context_line":"    return list(__instances_fill_metadata(context, instances, manual_joins))"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2F9UQ%3D","line":1584,"in_reply_to":"AAAAOn%2F%2F9bQ%3D","updated":"2013-04-10 07:02:33.000000000","message":"I think I agree with lifeless.  I don\u0027t see much use for the double underscore version outside of this.  This means the yield stuff goes away, I guess, since it should just return a list.","commit_id":"124e262705e3e41d4def96befc7fb2040948bb67"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5c30e9b42708ee6cecf1c7ef94aeb4500ac05ecc","unresolved":false,"context_lines":[{"line_number":1849,"context_line":""},{"line_number":1850,"context_line":"@require_admin_context"},{"line_number":1851,"context_line":"def instance_get_all_by_host_and_node(context, host, node):"},{"line_number":1852,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1853,"context_line":"        _instance_get_all_query(context, joins\u003d[]).filter_by(host\u003dhost)."},{"line_number":1854,"context_line":"            filter_by(node\u003dnode).all(), manual_joins\u003d[])"},{"line_number":1855,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8IA%3D","line":1852,"updated":"2013-04-10 20:15:39.000000000","message":"If manual_joins\u003d[] is _instances_fill_metadata necessary?  It will add the keys to the instance but I would think they aren\u0027t needed if the data isn\u0027t being used.","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"20049dc3dfb40bb39bcc76652eddcac8cbec9c12","unresolved":false,"context_lines":[{"line_number":1849,"context_line":""},{"line_number":1850,"context_line":"@require_admin_context"},{"line_number":1851,"context_line":"def instance_get_all_by_host_and_node(context, host, node):"},{"line_number":1852,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1853,"context_line":"        _instance_get_all_query(context, joins\u003d[]).filter_by(host\u003dhost)."},{"line_number":1854,"context_line":"            filter_by(node\u003dnode).all(), manual_joins\u003d[])"},{"line_number":1855,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8Ew%3D","line":1852,"in_reply_to":"AAAAOn%2F%2F8Go%3D","updated":"2013-04-10 20:46:14.000000000","message":"With the joins\u003d[] to _instance_get_all_query I wouldn\u0027t think so.  But I may be missing something.  Really not a big deal either way.","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a1291b47652d24a1f1241780622cd87ddc0e2e0","unresolved":false,"context_lines":[{"line_number":1849,"context_line":""},{"line_number":1850,"context_line":"@require_admin_context"},{"line_number":1851,"context_line":"def instance_get_all_by_host_and_node(context, host, node):"},{"line_number":1852,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1853,"context_line":"        _instance_get_all_query(context, joins\u003d[]).filter_by(host\u003dhost)."},{"line_number":1854,"context_line":"            filter_by(node\u003dnode).all(), manual_joins\u003d[])"},{"line_number":1855,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8Go%3D","line":1852,"in_reply_to":"AAAAOn%2F%2F8IA%3D","updated":"2013-04-10 20:22:57.000000000","message":"The act of serializing the SQLAlchemy Instance objects when they go over the wire will lazy-load these things, no?","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5c30e9b42708ee6cecf1c7ef94aeb4500ac05ecc","unresolved":false,"context_lines":[{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"@require_admin_context"},{"line_number":1858,"context_line":"def instance_get_all_by_host_and_not_type(context, host, type_id\u003dNone):"},{"line_number":1859,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1860,"context_line":"        _instance_get_all_query(context).filter_by(host\u003dhost)."},{"line_number":1861,"context_line":"                   filter(models.Instance.instance_type_id !\u003d type_id).all())"},{"line_number":1862,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8Io%3D","line":1859,"updated":"2013-04-10 20:15:39.000000000","message":"I don\u0027t think _instances_fill_metadata is needed here.  The only caller of instance_get_all_by_host_and_not_type that I see is a scheduler filter doing a length check on the number of returned instances.","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"20049dc3dfb40bb39bcc76652eddcac8cbec9c12","unresolved":false,"context_lines":[{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"@require_admin_context"},{"line_number":1858,"context_line":"def instance_get_all_by_host_and_not_type(context, host, type_id\u003dNone):"},{"line_number":1859,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1860,"context_line":"        _instance_get_all_query(context).filter_by(host\u003dhost)."},{"line_number":1861,"context_line":"                   filter(models.Instance.instance_type_id !\u003d type_id).all())"},{"line_number":1862,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8FQ%3D","line":1859,"in_reply_to":"AAAAOn%2F%2F8GY%3D","updated":"2013-04-10 20:46:14.000000000","message":"Ok.  Actually this call could leave out metadata, and all other joins in _instance_get_all_query.  But not a big deal for now.","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a1291b47652d24a1f1241780622cd87ddc0e2e0","unresolved":false,"context_lines":[{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"@require_admin_context"},{"line_number":1858,"context_line":"def instance_get_all_by_host_and_not_type(context, host, type_id\u003dNone):"},{"line_number":1859,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1860,"context_line":"        _instance_get_all_query(context).filter_by(host\u003dhost)."},{"line_number":1861,"context_line":"                   filter(models.Instance.instance_type_id !\u003d type_id).all())"},{"line_number":1862,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8GY%3D","line":1859,"in_reply_to":"AAAAOn%2F%2F8Io%3D","updated":"2013-04-10 20:22:57.000000000","message":"Okay, I hadn\u0027t audited all the uses, so I was trying to leave things unchanged unless the caller specifically asked for it. Brian had already determined that the get by host ones were good.","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5c30e9b42708ee6cecf1c7ef94aeb4500ac05ecc","unresolved":false,"context_lines":[{"line_number":1902,"context_line":"    reboot_window \u003d (timeutils.utcnow() -"},{"line_number":1903,"context_line":"                     datetime.timedelta(seconds\u003dreboot_window))"},{"line_number":1904,"context_line":""},{"line_number":1905,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1906,"context_line":"        model_query(context, models.Instance)."},{"line_number":1907,"context_line":"            filter(models.Instance.updated_at \u003c\u003d reboot_window)."},{"line_number":1908,"context_line":"            filter_by(task_state\u003dtask_states.REBOOTING).all())"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8Hc%3D","line":1905,"updated":"2013-04-10 20:15:39.000000000","message":"Is this actually necessary, or here to be safe?","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a1291b47652d24a1f1241780622cd87ddc0e2e0","unresolved":false,"context_lines":[{"line_number":1902,"context_line":"    reboot_window \u003d (timeutils.utcnow() -"},{"line_number":1903,"context_line":"                     datetime.timedelta(seconds\u003dreboot_window))"},{"line_number":1904,"context_line":""},{"line_number":1905,"context_line":"    return _instances_fill_metadata(context,"},{"line_number":1906,"context_line":"        model_query(context, models.Instance)."},{"line_number":1907,"context_line":"            filter(models.Instance.updated_at \u003c\u003d reboot_window)."},{"line_number":1908,"context_line":"            filter_by(task_state\u003dtask_states.REBOOTING).all())"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAOn%2F%2F8GM%3D","line":1905,"in_reply_to":"AAAAOn%2F%2F8Hc%3D","updated":"2013-04-10 20:22:57.000000000","message":"It\u0027s necessary because we pass the instances to the driver and I haven\u0027t audited all the paths to make sure it\u0027s save to exclude these. Again, just trying to make all of these smarter about keeping the same behavior until I know that they\u0027re save to optimize :)","commit_id":"c3568f9a1a28b269d88d2dcc175e6b5a2f7abf37"}]}