)]}'
{"nova/cells/manager.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f52c8c310c18a756fe886e7032c5d2cb486fd048","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                self.instances_to_heal \u003d cells_utils.get_instances_to_sync("},{"line_number":161,"context_line":"                        ctxt, updated_since\u003dupdated_since, shuffle\u003dTrue,"},{"line_number":162,"context_line":"                        uuids_only\u003dTrue,"},{"line_number":163,"context_line":"                        num_to_sync\u003dCONF.cells.instance_update_num_instances)"},{"line_number":164,"context_line":"                info[\u0027updated_list\u0027] \u003d True"},{"line_number":165,"context_line":"                try:"},{"line_number":166,"context_line":"                    instance \u003d self.instances_to_heal.next()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_7678eac4","line":163,"updated":"2015-05-16 05:57:07.000000000","message":"In get_instances_to_sync, it takes both num_to_sync and limit and takes the minimum of the two, but only num_to_sync is being passed here. It seems like no matter what you set here, if it\u0027s \u003e 100 it will get limited down to 100 inside get_instances_to_sync.","commit_id":"51a5c9d744697856e1017769f20cf02d51ef74e4"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"83c4ec3ae6c3b8e50f6ad154ab656fbbf07a1b71","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                self.instances_to_heal \u003d cells_utils.get_instances_to_sync("},{"line_number":161,"context_line":"                        ctxt, updated_since\u003dupdated_since, shuffle\u003dTrue,"},{"line_number":162,"context_line":"                        uuids_only\u003dTrue,"},{"line_number":163,"context_line":"                        num_to_sync\u003dCONF.cells.instance_update_num_instances)"},{"line_number":164,"context_line":"                info[\u0027updated_list\u0027] \u003d True"},{"line_number":165,"context_line":"                try:"},{"line_number":166,"context_line":"                    instance \u003d self.instances_to_heal.next()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_96ee9fb2","line":163,"in_reply_to":"7a016987_7678eac4","updated":"2015-05-18 21:12:30.000000000","message":"Right.  Limit is for protection against pulling too much data at once, and num_to_sync is intended as a hint in case the caller wants less than limit.  More of an optimization.  I can document this better, or remove it if that would be better.","commit_id":"51a5c9d744697856e1017769f20cf02d51ef74e4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ff1b6601739dd490c5bd8dca1dc803f38accd07","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                self.instances_to_heal \u003d cells_utils.get_instances_to_sync("},{"line_number":163,"context_line":"                        ctxt, updated_since\u003dupdated_since, shuffle\u003dTrue,"},{"line_number":164,"context_line":"                        uuids_only\u003dTrue,"},{"line_number":165,"context_line":"                        num_to_sync\u003dCONF.cells.instance_update_num_instances)"},{"line_number":166,"context_line":"                info[\u0027updated_list\u0027] \u003d True"},{"line_number":167,"context_line":"                try:"},{"line_number":168,"context_line":"                    instance \u003d next(self.instances_to_heal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a41bdd9_1a0d5f37","line":165,"updated":"2015-07-16 21:53:38.000000000","message":"The option name here \"instance_update_num_instances\" !\u003d the name in nova/cells/opts.py \"instance_update_database_limit\"\n\nBut it appears we don\u0027t need to specify num_to_sync at all, being that get_instances_to_sync itself pulls the conf setting as the default. It would make \"limit\" and \"num_to_sync\" be the same","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"48de908b4851aa9a9721acb6c90631787d1461e9","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                self.instances_to_heal \u003d cells_utils.get_instances_to_sync("},{"line_number":163,"context_line":"                        ctxt, updated_since\u003dupdated_since, shuffle\u003dTrue,"},{"line_number":164,"context_line":"                        uuids_only\u003dTrue,"},{"line_number":165,"context_line":"                        num_to_sync\u003dCONF.cells.instance_update_num_instances)"},{"line_number":166,"context_line":"                info[\u0027updated_list\u0027] \u003d True"},{"line_number":167,"context_line":"                try:"},{"line_number":168,"context_line":"                    instance \u003d next(self.instances_to_heal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_4a4bda87","line":165,"in_reply_to":"9a41bdd9_1a0d5f37","updated":"2015-08-27 18:58:43.000000000","message":"This is an optimization because instance_update_num_instances is by default a lower value than instance_update_database_limit.  It\u0027s used below on line 175.  So rather than pulling instances up to the database limit we can pull just what we need if it\u0027s lower than that.  This might be needless complexity though and makes the abstraction a bit leaky.","commit_id":"350329748f8e0063d8b99029900738083d5093f7"}],"nova/cells/opts.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ff1b6601739dd490c5bd8dca1dc803f38accd07","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    cfg.IntOpt(\u0027bandwidth_update_interval\u0027,"},{"line_number":55,"context_line":"                default\u003d600,"},{"line_number":56,"context_line":"                help\u003d\u0027Seconds between bandwidth updates for cells.\u0027),"},{"line_number":57,"context_line":"    cfg.IntOpt(\u0027instance_update_database_limit\u0027,"},{"line_number":58,"context_line":"            default\u003d100,"},{"line_number":59,"context_line":"            help\u003d\u0027Number of instance to pull from the database at one \u0027"},{"line_number":60,"context_line":"                 \u0027time for a sync.  If there are more instances to update \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a41bdd9_dad93785","line":57,"updated":"2015-07-16 21:53:38.000000000","message":"I\u0027m not sure if this name is too generic, that is, I wonder if it should have \"sync\" in it. Otherwise it sounds like it would apply to any bulk instance update in cells. But maybe the sync is the only bulk instance update and it\u0027s obvious? Not feeling strongly but wanted to mention it","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"48de908b4851aa9a9721acb6c90631787d1461e9","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    cfg.IntOpt(\u0027bandwidth_update_interval\u0027,"},{"line_number":55,"context_line":"                default\u003d600,"},{"line_number":56,"context_line":"                help\u003d\u0027Seconds between bandwidth updates for cells.\u0027),"},{"line_number":57,"context_line":"    cfg.IntOpt(\u0027instance_update_database_limit\u0027,"},{"line_number":58,"context_line":"            default\u003d100,"},{"line_number":59,"context_line":"            help\u003d\u0027Number of instance to pull from the database at one \u0027"},{"line_number":60,"context_line":"                 \u0027time for a sync.  If there are more instances to update \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_6d2a7880","line":57,"in_reply_to":"9a41bdd9_dad93785","updated":"2015-08-27 18:58:43.000000000","message":"I followed the style of instance_update_num_instances, but I agree.  I\u0027ll add sync in so it\u0027s more clear.","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ff1b6601739dd490c5bd8dca1dc803f38accd07","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                help\u003d\u0027Seconds between bandwidth updates for cells.\u0027),"},{"line_number":57,"context_line":"    cfg.IntOpt(\u0027instance_update_database_limit\u0027,"},{"line_number":58,"context_line":"            default\u003d100,"},{"line_number":59,"context_line":"            help\u003d\u0027Number of instance to pull from the database at one \u0027"},{"line_number":60,"context_line":"                 \u0027time for a sync.  If there are more instances to update \u0027"},{"line_number":61,"context_line":"                 \u0027the results will be paged through\u0027),"},{"line_number":62,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a41bdd9_9aedeff1","line":59,"range":{"start_line":59,"start_character":28,"end_line":59,"end_character":36},"updated":"2015-07-16 21:53:38.000000000","message":"typo","commit_id":"350329748f8e0063d8b99029900738083d5093f7"}],"nova/cells/utils.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f52c8c310c18a756fe886e7032c5d2cb486fd048","unresolved":false,"context_lines":[{"line_number":116,"context_line":""},{"line_number":117,"context_line":"def get_instances_to_sync(context, updated_since\u003dNone, project_id\u003dNone,"},{"line_number":118,"context_line":"        deleted\u003dTrue, shuffle\u003dFalse, uuids_only\u003dFalse, num_to_sync\u003dNone,"},{"line_number":119,"context_line":"        limit\u003d100):"},{"line_number":120,"context_line":"    \"\"\"Return a generator that will return a list of active and"},{"line_number":121,"context_line":"    deleted instances to sync with parent cells.  The list may"},{"line_number":122,"context_line":"    optionally be shuffled for periodic updates so that multiple"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_7626aacf","line":119,"updated":"2015-05-16 05:57:07.000000000","message":"This function is also called in nova/cells/messaging.py sync_instances. Is that one not intended to be configurable? (It will pull 100 at a time only)","commit_id":"51a5c9d744697856e1017769f20cf02d51ef74e4"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"83c4ec3ae6c3b8e50f6ad154ab656fbbf07a1b71","unresolved":false,"context_lines":[{"line_number":116,"context_line":""},{"line_number":117,"context_line":"def get_instances_to_sync(context, updated_since\u003dNone, project_id\u003dNone,"},{"line_number":118,"context_line":"        deleted\u003dTrue, shuffle\u003dFalse, uuids_only\u003dFalse, num_to_sync\u003dNone,"},{"line_number":119,"context_line":"        limit\u003d100):"},{"line_number":120,"context_line":"    \"\"\"Return a generator that will return a list of active and"},{"line_number":121,"context_line":"    deleted instances to sync with parent cells.  The list may"},{"line_number":122,"context_line":"    optionally be shuffled for periodic updates so that multiple"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_56082791","line":119,"in_reply_to":"7a016987_7626aacf","updated":"2015-05-18 21:12:30.000000000","message":"Good point.  I\u0027ll add some configurability via a config option to see how that looks.","commit_id":"51a5c9d744697856e1017769f20cf02d51ef74e4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ff1b6601739dd490c5bd8dca1dc803f38accd07","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"def get_instances_to_sync(context, updated_since\u003dNone, project_id\u003dNone,"},{"line_number":133,"context_line":"        deleted\u003dTrue, shuffle\u003dFalse, uuids_only\u003dFalse, num_to_sync\u003dNone):"},{"line_number":134,"context_line":"    \"\"\"Return a generator that will return a list of active and"},{"line_number":135,"context_line":"    deleted instances to sync with parent cells.  The list may"},{"line_number":136,"context_line":"    optionally be shuffled for periodic updates so that multiple"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a41bdd9_4b2f9bf6","line":133,"updated":"2015-07-16 21:53:38.000000000","message":"Do we need num_to_sync at all if we\u0027re always going to pass in the conf setting value? Or are we adding it to leave it open for potential future calls?","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"48de908b4851aa9a9721acb6c90631787d1461e9","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"def get_instances_to_sync(context, updated_since\u003dNone, project_id\u003dNone,"},{"line_number":133,"context_line":"        deleted\u003dTrue, shuffle\u003dFalse, uuids_only\u003dFalse, num_to_sync\u003dNone):"},{"line_number":134,"context_line":"    \"\"\"Return a generator that will return a list of active and"},{"line_number":135,"context_line":"    deleted instances to sync with parent cells.  The list may"},{"line_number":136,"context_line":"    optionally be shuffled for periodic updates so that multiple"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_ad47304a","line":133,"in_reply_to":"9a41bdd9_4b2f9bf6","updated":"2015-08-27 18:58:43.000000000","message":"As mentioned in another response, it\u0027s an optimization.  But it\u0027s not done in a clear way, and I\u0027m now thinking it requires callers to care too much about implementation details of this method.  I\u0027m going to remove it.","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ff1b6601739dd490c5bd8dca1dc803f38accd07","unresolved":false,"context_lines":[{"line_number":163,"context_line":"    if not deleted:"},{"line_number":164,"context_line":"        filters[\u0027deleted\u0027] \u003d False"},{"line_number":165,"context_line":"    # Active instances first."},{"line_number":166,"context_line":"    limit \u003d CONF.cells.instance_update_database_limit"},{"line_number":167,"context_line":"    limit \u003d min(limit, num_to_sync) if num_to_sync is not None else limit"},{"line_number":168,"context_line":"    marker \u003d None"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a41bdd9_6b4dbfc3","line":166,"updated":"2015-07-16 21:53:38.000000000","message":"This is the default I was talking about in nova/cells/manager.py","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"95c599b26fbbe5ac3471078c239a2267cd711222","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if shuffle:"},{"line_number":146,"context_line":"                # NOTE(melwitt): Need a list that supports assignment for"},{"line_number":147,"context_line":"                # shuffle."},{"line_number":148,"context_line":"                instances \u003d [instance for instance in instances]"},{"line_number":149,"context_line":"                random.shuffle(instances)"},{"line_number":150,"context_line":"        return instances, marker"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_79ed4eb6","line":148,"range":{"start_line":148,"start_character":28,"end_line":148,"end_character":64},"updated":"2015-09-22 10:51:52.000000000","message":"curious: doesn\u0027t list(instances) work?","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"55d6d62297244cbf26cc7eb8a822a492a0f6e63f","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if shuffle:"},{"line_number":146,"context_line":"                # NOTE(melwitt): Need a list that supports assignment for"},{"line_number":147,"context_line":"                # shuffle."},{"line_number":148,"context_line":"                instances \u003d [instance for instance in instances]"},{"line_number":149,"context_line":"                random.shuffle(instances)"},{"line_number":150,"context_line":"        return instances, marker"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_11217afb","line":148,"range":{"start_line":148,"start_character":28,"end_line":148,"end_character":64},"in_reply_to":"ba15a1d1_0b04edbd","updated":"2015-09-22 15:03:42.000000000","message":"Good to know :)\n\nI updated this locally to be list(instances) and updated to tests to exercise this with InstanceList objects and everything works great.","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"1c40089eaddc90eabc2c513c27086f014b90235f","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if shuffle:"},{"line_number":146,"context_line":"                # NOTE(melwitt): Need a list that supports assignment for"},{"line_number":147,"context_line":"                # shuffle."},{"line_number":148,"context_line":"                instances \u003d [instance for instance in instances]"},{"line_number":149,"context_line":"                random.shuffle(instances)"},{"line_number":150,"context_line":"        return instances, marker"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_5027f011","line":148,"range":{"start_line":148,"start_character":28,"end_line":148,"end_character":64},"in_reply_to":"ba15a1d1_254560b2","updated":"2015-09-22 14:12:19.000000000","message":"Testing:\n\n \u003e\u003e\u003e import random as r\n \u003e\u003e\u003e a \u003d \"123\"\n \u003e\u003e\u003e r.shuffle(a)\n Traceback (most recent call last):\n   File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n   File \"/usr/lib/python2.7/random.py\", line 291, in shuffle\n     x[i], x[j] \u003d x[j], x[i]\n TypeError: \u0027str\u0027 object does not support item assignment\n \u003e\u003e\u003e a \u003d list(\"123\")\n \u003e\u003e\u003e r.shuffle(a)\n \u003e\u003e\u003e a\n [\u00273\u0027, \u00271\u0027, \u00272\u0027]","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"5c5cd732c1ed784df95105dcbc85ce6c03e2c13a","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if shuffle:"},{"line_number":146,"context_line":"                # NOTE(melwitt): Need a list that supports assignment for"},{"line_number":147,"context_line":"                # shuffle."},{"line_number":148,"context_line":"                instances \u003d [instance for instance in instances]"},{"line_number":149,"context_line":"                random.shuffle(instances)"},{"line_number":150,"context_line":"        return instances, marker"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_0b04edbd","line":148,"range":{"start_line":148,"start_character":28,"end_line":148,"end_character":64},"in_reply_to":"ba15a1d1_2bb53130","updated":"2015-09-22 14:23:49.000000000","message":":)\n\n \u003e\u003e\u003e import collections as c\n \u003e\u003e\u003e isinstance(\"\", c.Sequence)\n True","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3cae6f8c3637f5d22b340c66b4d9d73cd4f03bb2","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if shuffle:"},{"line_number":146,"context_line":"                # NOTE(melwitt): Need a list that supports assignment for"},{"line_number":147,"context_line":"                # shuffle."},{"line_number":148,"context_line":"                instances \u003d [instance for instance in instances]"},{"line_number":149,"context_line":"                random.shuffle(instances)"},{"line_number":150,"context_line":"        return instances, marker"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_2bb53130","line":148,"range":{"start_line":148,"start_character":28,"end_line":148,"end_character":64},"in_reply_to":"ba15a1d1_5027f011","updated":"2015-09-22 14:20:20.000000000","message":"The difference in this case, and I\u0027m not sure if it matters, is that instances is an object that inherits from the collections.Sequence ABC.  But I\u0027ll test it when I rework the patch.","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"619eede6c1be738622ba04d7886028510c7c7067","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if shuffle:"},{"line_number":146,"context_line":"                # NOTE(melwitt): Need a list that supports assignment for"},{"line_number":147,"context_line":"                # shuffle."},{"line_number":148,"context_line":"                instances \u003d [instance for instance in instances]"},{"line_number":149,"context_line":"                random.shuffle(instances)"},{"line_number":150,"context_line":"        return instances, marker"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_254560b2","line":148,"range":{"start_line":148,"start_character":28,"end_line":148,"end_character":64},"in_reply_to":"ba15a1d1_79ed4eb6","updated":"2015-09-22 13:52:01.000000000","message":"It seems like it might, but I would defer to melwitt to answer that since she likely learned that NOTE the hard way.","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"95c599b26fbbe5ac3471078c239a2267cd711222","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    instances \u003d []"},{"line_number":164,"context_line":"    while True:"},{"line_number":165,"context_line":"        if len(instances) \u003d\u003d 0:"},{"line_number":166,"context_line":"            instances, marker \u003d _get_paginated_instances(context, filters,"},{"line_number":167,"context_line":"                    shuffle, limit, marker)"},{"line_number":168,"context_line":"        if len(instances) \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_9cf930cb","line":165,"range":{"start_line":165,"start_character":11,"end_line":165,"end_character":30},"updated":"2015-09-22 10:51:52.000000000","message":"+0: Honestly I\u0027m fine with this, but I\u0027ve had reviews saying this should be \"if not instances\"","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"619eede6c1be738622ba04d7886028510c7c7067","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    instances \u003d []"},{"line_number":164,"context_line":"    while True:"},{"line_number":165,"context_line":"        if len(instances) \u003d\u003d 0:"},{"line_number":166,"context_line":"            instances, marker \u003d _get_paginated_instances(context, filters,"},{"line_number":167,"context_line":"                    shuffle, limit, marker)"},{"line_number":168,"context_line":"        if len(instances) \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_65c0387d","line":165,"range":{"start_line":165,"start_character":11,"end_line":165,"end_character":30},"in_reply_to":"ba15a1d1_9cf930cb","updated":"2015-09-22 13:52:01.000000000","message":"That seems odd to me.  If instances could be None or a list that makes sense but here instances is always a list.  I may change it though.","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"95c599b26fbbe5ac3471078c239a2267cd711222","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                    shuffle, limit, marker)"},{"line_number":168,"context_line":"        if len(instances) \u003d\u003d 0:"},{"line_number":169,"context_line":"            break"},{"line_number":170,"context_line":"        instance \u003d instances.pop()"},{"line_number":171,"context_line":"        if uuids_only:"},{"line_number":172,"context_line":"            yield instance.uuid"},{"line_number":173,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_9c20504b","line":170,"range":{"start_line":170,"start_character":29,"end_line":170,"end_character":34},"updated":"2015-09-22 10:51:52.000000000","message":"-1: If 1..9 gets grouped by 3, you\u0027ll traverse [3,2,1,6,5,4,9,8,7]. Maybe use pop(0)?","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"619eede6c1be738622ba04d7886028510c7c7067","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                    shuffle, limit, marker)"},{"line_number":168,"context_line":"        if len(instances) \u003d\u003d 0:"},{"line_number":169,"context_line":"            break"},{"line_number":170,"context_line":"        instance \u003d instances.pop()"},{"line_number":171,"context_line":"        if uuids_only:"},{"line_number":172,"context_line":"            yield instance.uuid"},{"line_number":173,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba15a1d1_05bffcfb","line":170,"range":{"start_line":170,"start_character":29,"end_line":170,"end_character":34},"in_reply_to":"ba15a1d1_9c20504b","updated":"2015-09-22 13:52:01.000000000","message":"Sure.","commit_id":"85e297a616319c1af472e42c5341eb1aff357326"}],"nova/tests/unit/cells/test_cells_utils.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f52c8c310c18a756fe886e7032c5d2cb486fd048","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            call_info[\u0027got_filters\u0027] \u003d filters"},{"line_number":114,"context_line":"            instances \u003d [fake_instance.fake_db_instance() for i in range(3)]"},{"line_number":115,"context_line":"            call_info[\u0027marker\u0027] \u003d instances[2][\u0027uuid\u0027]"},{"line_number":116,"context_line":"            return instances"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        self.stubs.Set(db, \u0027instance_get_all_by_filters\u0027,"},{"line_number":119,"context_line":"                instance_get_all_by_filters)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_31c5dc3b","line":116,"updated":"2015-05-16 05:57:07.000000000","message":"Instead of doing logic to figure what to return for each call, you can use mock and use side_effect \u003d [first return value, second return value, third return value] [1]. Then use call_args_list [2] to assert the args are right for each call.\n\n[1] https://mock.readthedocs.org/en/latest/mock.html?highlight\u003dassert_called_with#mock.Mock.side_effect\n[2] https://mock.readthedocs.org/en/latest/mock.html?highlight\u003dassert_called_with#mock.Mock.call_args_list","commit_id":"51a5c9d744697856e1017769f20cf02d51ef74e4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f52c8c310c18a756fe886e7032c5d2cb486fd048","unresolved":false,"context_lines":[{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        self.stubs.Set(db, \u0027instance_get_all_by_filters\u0027,"},{"line_number":119,"context_line":"                instance_get_all_by_filters)"},{"line_number":120,"context_line":"        self.stubs.Set(random, \u0027shuffle\u0027, random_shuffle)"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        instances \u003d cells_utils.get_instances_to_sync(fake_context)"},{"line_number":123,"context_line":"        self.assertTrue(inspect.isgenerator(instances))"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_71cfd47f","line":120,"updated":"2015-05-16 05:57:07.000000000","message":"Prefer to use mock and get call_count from it. That way you don\u0027t have to make a fake method for it at all.","commit_id":"51a5c9d744697856e1017769f20cf02d51ef74e4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ff1b6601739dd490c5bd8dca1dc803f38accd07","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            call_info[\u0027got_filters\u0027] \u003d filters"},{"line_number":50,"context_line":"            call_info[\u0027get_all\u0027] +\u003d 1"},{"line_number":51,"context_line":"            instances \u003d [fake_instance.fake_db_instance() for i in range(3)]"},{"line_number":52,"context_line":"            call_info[\u0027inst_marker\u0027] \u003d instances[2][\u0027uuid\u0027]"},{"line_number":53,"context_line":"            return instances"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        self.stubs.Set(objects.InstanceList, \u0027get_by_filters\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a41bdd9_868cba80","line":52,"updated":"2015-07-16 21:53:38.000000000","message":"I think you don\u0027t need this, it\u0027s not used anywhere and you\u0027re checking the markers with assert_has_calls","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"48de908b4851aa9a9721acb6c90631787d1461e9","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            call_info[\u0027got_filters\u0027] \u003d filters"},{"line_number":50,"context_line":"            call_info[\u0027get_all\u0027] +\u003d 1"},{"line_number":51,"context_line":"            instances \u003d [fake_instance.fake_db_instance() for i in range(3)]"},{"line_number":52,"context_line":"            call_info[\u0027inst_marker\u0027] \u003d instances[2][\u0027uuid\u0027]"},{"line_number":53,"context_line":"            return instances"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        self.stubs.Set(objects.InstanceList, \u0027get_by_filters\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_8dab4c1d","line":52,"in_reply_to":"9a41bdd9_868cba80","updated":"2015-08-27 18:58:43.000000000","message":"Good catch.  Probably left over from something I tried previously.","commit_id":"350329748f8e0063d8b99029900738083d5093f7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"92d3c044f6a5b173dc3e239f1ba45b30fb90fe47","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        @staticmethod"},{"line_number":40,"context_line":"        def instance_get_all_by_filters(context, filters,"},{"line_number":41,"context_line":"                sort_key, sort_dir, limit, marker):"},{"line_number":42,"context_line":"            # Pretend we return a full list the first time otherwise we loop"},{"line_number":43,"context_line":"            # infinitely"},{"line_number":44,"context_line":"            if marker is not None:"},{"line_number":45,"context_line":"                return []"},{"line_number":46,"context_line":"            self.assertEqual(fake_context, context)"},{"line_number":47,"context_line":"            self.assertEqual(\u0027deleted\u0027, sort_key)"},{"line_number":48,"context_line":"            self.assertEqual(\u0027asc\u0027, sort_dir)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_88eeb189","line":45,"range":{"start_line":42,"start_character":12,"end_line":45,"end_character":25},"updated":"2015-09-22 19:52:13.000000000","message":"Is this needed for already existing tests to continue working? Or is it a remnant of an earlier version of the pagination tests?","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"bad7ef7fb425bd070943138b5bb67a9b845f7657","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        @staticmethod"},{"line_number":40,"context_line":"        def instance_get_all_by_filters(context, filters,"},{"line_number":41,"context_line":"                sort_key, sort_dir, limit, marker):"},{"line_number":42,"context_line":"            # Pretend we return a full list the first time otherwise we loop"},{"line_number":43,"context_line":"            # infinitely"},{"line_number":44,"context_line":"            if marker is not None:"},{"line_number":45,"context_line":"                return []"},{"line_number":46,"context_line":"            self.assertEqual(fake_context, context)"},{"line_number":47,"context_line":"            self.assertEqual(\u0027deleted\u0027, sort_key)"},{"line_number":48,"context_line":"            self.assertEqual(\u0027asc\u0027, sort_dir)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_b3b2d694","line":45,"range":{"start_line":42,"start_character":12,"end_line":45,"end_character":25},"in_reply_to":"ba15a1d1_88eeb189","updated":"2015-09-23 17:49:55.000000000","message":"This is still needed, otherwise the pagination gets into an infinite loop calling this.","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"be9d932f1a2f912e7b76433c1ded3b7217563470","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        @staticmethod"},{"line_number":40,"context_line":"        def instance_get_all_by_filters(context, filters,"},{"line_number":41,"context_line":"                sort_key, sort_dir, limit, marker):"},{"line_number":42,"context_line":"            # Pretend we return a full list the first time otherwise we loop"},{"line_number":43,"context_line":"            # infinitely"},{"line_number":44,"context_line":"            if marker is not None:"},{"line_number":45,"context_line":"                return []"},{"line_number":46,"context_line":"            self.assertEqual(fake_context, context)"},{"line_number":47,"context_line":"            self.assertEqual(\u0027deleted\u0027, sort_key)"},{"line_number":48,"context_line":"            self.assertEqual(\u0027asc\u0027, sort_dir)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_1e31495a","line":45,"range":{"start_line":42,"start_character":12,"end_line":45,"end_character":25},"in_reply_to":"ba15a1d1_b3b2d694","updated":"2015-09-23 18:08:15.000000000","message":"My bad, I was missing that paginated query has to make two calls, minimum.","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"92d3c044f6a5b173dc3e239f1ba45b30fb90fe47","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            call_info[\u0027got_filters\u0027] \u003d filters"},{"line_number":50,"context_line":"            call_info[\u0027get_all\u0027] +\u003d 1"},{"line_number":51,"context_line":"            instances \u003d [fake_instance.fake_db_instance() for i in range(3)]"},{"line_number":52,"context_line":"            return instances"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        self.stubs.Set(objects.InstanceList, \u0027get_by_filters\u0027,"},{"line_number":55,"context_line":"                instance_get_all_by_filters)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_8863111a","line":52,"updated":"2015-09-22 19:52:13.000000000","message":"General question: are any of the changes to this function needed anymore? Asking because of the side_effect used for the pagination tests.","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a3f3840e2c6d7cb23d8fdcc2488d572c7e57ece3","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            call_info[\u0027got_filters\u0027] \u003d filters"},{"line_number":50,"context_line":"            call_info[\u0027get_all\u0027] +\u003d 1"},{"line_number":51,"context_line":"            instances \u003d [fake_instance.fake_db_instance() for i in range(3)]"},{"line_number":52,"context_line":"            return instances"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        self.stubs.Set(objects.InstanceList, \u0027get_by_filters\u0027,"},{"line_number":55,"context_line":"                instance_get_all_by_filters)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_0314c241","line":52,"in_reply_to":"ba15a1d1_8863111a","updated":"2015-09-22 19:53:16.000000000","message":"Changes other than limit and marker added to the arguments, I mean.","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"bad7ef7fb425bd070943138b5bb67a9b845f7657","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            call_info[\u0027got_filters\u0027] \u003d filters"},{"line_number":50,"context_line":"            call_info[\u0027get_all\u0027] +\u003d 1"},{"line_number":51,"context_line":"            instances \u003d [fake_instance.fake_db_instance() for i in range(3)]"},{"line_number":52,"context_line":"            return instances"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        self.stubs.Set(objects.InstanceList, \u0027get_by_filters\u0027,"},{"line_number":55,"context_line":"                instance_get_all_by_filters)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_d36e6aff","line":52,"in_reply_to":"ba15a1d1_8863111a","updated":"2015-09-23 17:49:55.000000000","message":"This is still needed because the pagination method accesses \u0027uuid\u0027 on the instance so the fake strings from before won\u0027t work.","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"92d3c044f6a5b173dc3e239f1ba45b30fb90fe47","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        instances \u003d cells_utils.get_instances_to_sync(fake_context,"},{"line_number":113,"context_line":"                updated_since, project_id, shuffle\u003dshuffle)"},{"line_number":114,"context_line":"        self.assertEqual(len([x for x in instances]), 6)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        filters \u003d {}"},{"line_number":117,"context_line":"        if updated_since is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_685b1df7","line":114,"range":{"start_line":114,"start_character":29,"end_line":114,"end_character":51},"updated":"2015-09-22 19:52:13.000000000","message":"Can this not just be \"instances\"? Or based on the other change it would be list(instances)","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"bad7ef7fb425bd070943138b5bb67a9b845f7657","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        instances \u003d cells_utils.get_instances_to_sync(fake_context,"},{"line_number":113,"context_line":"                updated_since, project_id, shuffle\u003dshuffle)"},{"line_number":114,"context_line":"        self.assertEqual(len([x for x in instances]), 6)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        filters \u003d {}"},{"line_number":117,"context_line":"        if updated_since is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_73cf5e7f","line":114,"range":{"start_line":114,"start_character":29,"end_line":114,"end_character":51},"in_reply_to":"ba15a1d1_685b1df7","updated":"2015-09-23 17:49:55.000000000","message":"Yep.  Not sure why I did this.","commit_id":"a8a349c10f4309a43c17a483bdf3718fa46e4bd3"}]}