)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"ef8eee06bf8604cccf19e09bb390b6429edfebd4","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Joshua Harlow \u003cjxharlow@godaddy.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-12-08 15:39:46 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"The implementation of list servers cache is wrong"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The cache needs to actually take into account the"},{"line_number":10,"context_line":"params passed to list_servers and store those"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"df87a7cf_291a35cb","line":7,"updated":"2017-12-09 14:17:49.000000000","message":"Nit - but since it needs to be respun anyway ... commit subject summarizing what the patch did, not what the problem  is would be better. Try: \"Make list_servers cache honor params\"","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"}],"shade/openstackcloud.py":[{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"886a16e86d7fe4a1d955818a1aad7e56d114959f","unresolved":false,"context_lines":[{"line_number":2163,"context_line":"        # on *all* the input params here, because the server cache we use"},{"line_number":2164,"context_line":"        # needs to be based on all the params, not just one potential"},{"line_number":2165,"context_line":"        # variation; otherwise the internal cache is wrong."},{"line_number":2166,"context_line":"        cache_key \u003d (detailed, all_projects, bare, filters)"},{"line_number":2167,"context_line":"        cache_key_lock \u003d self._servers_cache_locks.setdefault("},{"line_number":2168,"context_line":"            cache_key, threading.Lock())"},{"line_number":2169,"context_line":"        old_cached_val \u003d self._servers_cache.get(cache_key)"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_89a581a4","line":2166,"updated":"2017-12-09 14:00:00.000000000","message":"With this change to consider parameters, we need to reorganize just a little and shift the caching into being around the get call.\n\nI think that ACTUALLY means we want to shift a little of the logic that\u0027s in _list_servers into this call so that it\u0027s clear what we\u0027re caching and what we\u0027re not.\n\nSee below - but we don\u0027t need to consider detailed and bare in the caching - they control client-side post-processing. That post-processing may make a few additional calls, but those calls have their own resource caching.","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"886a16e86d7fe4a1d955818a1aad7e56d114959f","unresolved":false,"context_lines":[{"line_number":2163,"context_line":"        # on *all* the input params here, because the server cache we use"},{"line_number":2164,"context_line":"        # needs to be based on all the params, not just one potential"},{"line_number":2165,"context_line":"        # variation; otherwise the internal cache is wrong."},{"line_number":2166,"context_line":"        cache_key \u003d (detailed, all_projects, bare, filters)"},{"line_number":2167,"context_line":"        cache_key_lock \u003d self._servers_cache_locks.setdefault("},{"line_number":2168,"context_line":"            cache_key, threading.Lock())"},{"line_number":2169,"context_line":"        old_cached_val \u003d self._servers_cache.get(cache_key)"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_29e315db","line":2166,"range":{"start_line":2166,"start_character":51,"end_line":2166,"end_character":58},"updated":"2017-12-09 14:00:00.000000000","message":"filters is a dict, which is leading to the test failure.\n\nWe have a method, _make_cache_key, that has an inner closure function generate_key that we use for the dogpile version of this. I think we could extract generate_keys into a normal function (accounting for name_key and fname being None potentially) and then use generate_key here with filters passed to it as the kwargs value. Except actually with params passed, due to the whole \"let\u0027s cache the get call, not the post-process of the get call\"","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"886a16e86d7fe4a1d955818a1aad7e56d114959f","unresolved":false,"context_lines":[{"line_number":2164,"context_line":"        # needs to be based on all the params, not just one potential"},{"line_number":2165,"context_line":"        # variation; otherwise the internal cache is wrong."},{"line_number":2166,"context_line":"        cache_key \u003d (detailed, all_projects, bare, filters)"},{"line_number":2167,"context_line":"        cache_key_lock \u003d self._servers_cache_locks.setdefault("},{"line_number":2168,"context_line":"            cache_key, threading.Lock())"},{"line_number":2169,"context_line":"        old_cached_val \u003d self._servers_cache.get(cache_key)"},{"line_number":2170,"context_line":"        got_lock \u003d cache_key_lock.acquire(False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_496c599b","line":2167,"updated":"2017-12-09 14:00:00.000000000","message":"Is weakref.WeakValueDictionary.setdefault threadsafe? If so, awesome, this is a nice pattern.","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"239c5bb576a1414a827d6b67d1c2cfc9f08466c1","unresolved":false,"context_lines":[{"line_number":2164,"context_line":"        # needs to be based on all the params, not just one potential"},{"line_number":2165,"context_line":"        # variation; otherwise the internal cache is wrong."},{"line_number":2166,"context_line":"        cache_key \u003d (detailed, all_projects, bare, filters)"},{"line_number":2167,"context_line":"        cache_key_lock \u003d self._servers_cache_locks.setdefault("},{"line_number":2168,"context_line":"            cache_key, threading.Lock())"},{"line_number":2169,"context_line":"        old_cached_val \u003d self._servers_cache.get(cache_key)"},{"line_number":2170,"context_line":"        got_lock \u003d cache_key_lock.acquire(False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_e54692c0","line":2167,"in_reply_to":"df87a7cf_496c599b","updated":"2017-12-11 18:21:26.000000000","message":"Yes, afaik, will double check though.","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"0ab4ba13818c0fa89de34f39a51fdb1d581d9103","unresolved":false,"context_lines":[{"line_number":2178,"context_line":"        try:"},{"line_number":2179,"context_line":"            servers \u003d None"},{"line_number":2180,"context_line":"            try:"},{"line_number":2181,"context_line":"                servers, saved_time \u003d self._servers_cache[cache_key]"},{"line_number":2182,"context_line":"                if (time.time() - saved_time) \u003e\u003d self._SERVER_AGE:"},{"line_number":2183,"context_line":"                    servers \u003d None"},{"line_number":2184,"context_line":"            except KeyError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_9d3864a0","line":2181,"updated":"2017-12-09 00:45:33.000000000","message":"Exceptions are expensive. We expect this as a normal result, so how about this instead:\n\n    servers, saved_time \u003d self._servers_cache.get(cache_key, (None, None))\n    if saved_time is not None and (time.time() - saved_time) \u003e\u003d self._SERVER_AGE:\n\nAnd then we can drop the try/except.","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"2825359af64da84bcfb691b1913ff11536e39b6d","unresolved":false,"context_lines":[{"line_number":2178,"context_line":"        try:"},{"line_number":2179,"context_line":"            servers \u003d None"},{"line_number":2180,"context_line":"            try:"},{"line_number":2181,"context_line":"                servers, saved_time \u003d self._servers_cache[cache_key]"},{"line_number":2182,"context_line":"                if (time.time() - saved_time) \u003e\u003d self._SERVER_AGE:"},{"line_number":2183,"context_line":"                    servers \u003d None"},{"line_number":2184,"context_line":"            except KeyError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_fd989075","line":2181,"in_reply_to":"df87a7cf_9d3864a0","updated":"2017-12-09 01:12:51.000000000","message":"I\u0027m fine with that.","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"886a16e86d7fe4a1d955818a1aad7e56d114959f","unresolved":false,"context_lines":[{"line_number":2199,"context_line":"            cloud\u003dself.name,"},{"line_number":2200,"context_line":"            region\u003dself.region_name)"},{"line_number":2201,"context_line":""},{"line_number":2202,"context_line":"        params \u003d filters or {}"},{"line_number":2203,"context_line":"        if all_projects:"},{"line_number":2204,"context_line":"            params[\u0027all_tenants\u0027] \u003d True"},{"line_number":2205,"context_line":"        data \u003d self._compute_client.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_09a411b9","line":2202,"updated":"2017-12-09 14:00:00.000000000","message":"There is another error in here that I just noticed. If we\u0027re in batched operation mode (that is, if self._SERVER_AGE \u003e 0,  we should not be pushing filters to nova. (see self.list_floating_ips) The reason for that is that in batched mode, we want to get the full list once and only once every SERVER_AGE and filter client-side.\n\nIn this case AND in the floating ips case, we shoud also honor self.use_direct_get. use_direct_get is a flag that can be specified to tell shade to prefer pushdown over client-side filtering.\n\nSo I think this should be:\n\n  if self._SERVER_AGE \u003d\u003d 0 or self.use_direct_get:\n    params \u003d filters\n  else:\n    params \u003d {}","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"886a16e86d7fe4a1d955818a1aad7e56d114959f","unresolved":false,"context_lines":[{"line_number":2202,"context_line":"        params \u003d filters or {}"},{"line_number":2203,"context_line":"        if all_projects:"},{"line_number":2204,"context_line":"            params[\u0027all_tenants\u0027] \u003d True"},{"line_number":2205,"context_line":"        data \u003d self._compute_client.get("},{"line_number":2206,"context_line":"            \u0027/servers/detail\u0027, params\u003dparams, error_message\u003derror_msg)"},{"line_number":2207,"context_line":"        servers \u003d self._normalize_servers("},{"line_number":2208,"context_line":"            self._get_and_munchify(\u0027servers\u0027, data))"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_c9b4e9f4","line":2205,"updated":"2017-12-09 14:00:00.000000000","message":"This is the call we should actually be caching, with a cache key based on params. With the other logic correct, if we cache based on params, that\u0027ll take care of all_projects and filters, and it\u0027ll automatically do the right thing for normal or batched operation.\n\nDetailed and bare have to do with post-processing done on the list afterwards, not on the call to /servers/detail - we don\u0027t need to cache them with the servers cache.","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"886a16e86d7fe4a1d955818a1aad7e56d114959f","unresolved":false,"context_lines":[{"line_number":2204,"context_line":"            params[\u0027all_tenants\u0027] \u003d True"},{"line_number":2205,"context_line":"        data \u003d self._compute_client.get("},{"line_number":2206,"context_line":"            \u0027/servers/detail\u0027, params\u003dparams, error_message\u003derror_msg)"},{"line_number":2207,"context_line":"        servers \u003d self._normalize_servers("},{"line_number":2208,"context_line":"            self._get_and_munchify(\u0027servers\u0027, data))"},{"line_number":2209,"context_line":"        return ["},{"line_number":2210,"context_line":"            self._expand_server(server, detailed, bare)"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_e9ac4da0","line":2207,"updated":"2017-12-09 14:00:00.000000000","message":"Then here it should be:\n\n  servers \u003d self._normalize_servers(\n    _utils._filter_list(\n        self._get_and_munchify(\u0027servers\u0027, data),\n        None, filters))\n\nSo that we do any client-side filtering that needs to be done. It technically COULD be wrapped in an if, like:\n\n  if self._SERVER_AGE \u003d\u003d 0 or self.use_direct_get:\n    servers \u003d self._normalize_servers(\n        self._get_and_munchify(\u0027servers\u0027, data),\n          None, filters)\n  else:\n    servers \u003d self._normalize_servers(\n      _utils._filter_list(\n          self._get_and_munchify(\u0027servers\u0027, data),\n          None, filters))\n\nbut filter_list is quick and will be a no-op if we\u0027ve pushed filters to the server, so I think the if block here would be a waste.","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"239c5bb576a1414a827d6b67d1c2cfc9f08466c1","unresolved":false,"context_lines":[{"line_number":2204,"context_line":"            params[\u0027all_tenants\u0027] \u003d True"},{"line_number":2205,"context_line":"        data \u003d self._compute_client.get("},{"line_number":2206,"context_line":"            \u0027/servers/detail\u0027, params\u003dparams, error_message\u003derror_msg)"},{"line_number":2207,"context_line":"        servers \u003d self._normalize_servers("},{"line_number":2208,"context_line":"            self._get_and_munchify(\u0027servers\u0027, data))"},{"line_number":2209,"context_line":"        return ["},{"line_number":2210,"context_line":"            self._expand_server(server, detailed, bare)"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_650cc293","line":2207,"in_reply_to":"df87a7cf_e9ac4da0","updated":"2017-12-11 18:21:26.000000000","message":"Okies, ha, shade is doing to much caching, lol","commit_id":"f957db9661dbc57da7be197cd6f76410065b269a"}]}