)]}'
{"nova/conductor/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e2fa06dfb684316b81be6b29bf2204b031ac005b","unresolved":false,"context_lines":[{"line_number":1668,"context_line":"                  \u0027cells\u0027: len(hosts_by_cell)})"},{"line_number":1669,"context_line":"        clock.start()"},{"line_number":1670,"context_line":""},{"line_number":1671,"context_line":"        stats \u003d collections.defaultdict(lambda: (0, 0, 0))"},{"line_number":1672,"context_line":"        failed_images \u003d collections.defaultdict(int)"},{"line_number":1673,"context_line":""},{"line_number":1674,"context_line":"        def wrap_cache_images(context, host, image_ids):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_fcee541f","line":1671,"range":{"start_line":1671,"start_character":40,"end_line":1671,"end_character":57},"updated":"2019-10-28 19:50:24.000000000","message":"A namedtuple might be a bit clearer usage, except the defaults kwarg for namedtuple wasn\u0027t added until 3.7 so you can\u0027t default the fields to 0 unless you subclass or something at which point it loses value in simplicity.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e2fa06dfb684316b81be6b29bf2204b031ac005b","unresolved":false,"context_lines":[{"line_number":1671,"context_line":"        stats \u003d collections.defaultdict(lambda: (0, 0, 0))"},{"line_number":1672,"context_line":"        failed_images \u003d collections.defaultdict(int)"},{"line_number":1673,"context_line":""},{"line_number":1674,"context_line":"        def wrap_cache_images(context, host, image_ids):"},{"line_number":1675,"context_line":"            result \u003d self.compute_rpcapi.cache_images("},{"line_number":1676,"context_line":"                target_ctxt,"},{"line_number":1677,"context_line":"                host\u003dhost,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_fc797481","line":1674,"range":{"start_line":1674,"start_character":30,"end_line":1674,"end_character":37},"updated":"2019-10-28 19:50:24.000000000","message":"This isn\u0027t used. It should be target_ctxt and it works because target_ctxt is defined on L1692 before it\u0027s used here, but is that correct if we\u0027re iterating multiple cells and using spawn_n? Meaning could we call self.compute_rpcapi.cache_images with the wrong target_ctxt if the target_ctxt changed before spawn_n calls the method? At the very least it\u0027s confusing to have this \u0027context\u0027 arg but not use it here.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"182273d746fd15148bee217fd09ead61b20c678b","unresolved":false,"context_lines":[{"line_number":1671,"context_line":"        stats \u003d collections.defaultdict(lambda: (0, 0, 0))"},{"line_number":1672,"context_line":"        failed_images \u003d collections.defaultdict(int)"},{"line_number":1673,"context_line":""},{"line_number":1674,"context_line":"        def wrap_cache_images(context, host, image_ids):"},{"line_number":1675,"context_line":"            result \u003d self.compute_rpcapi.cache_images("},{"line_number":1676,"context_line":"                target_ctxt,"},{"line_number":1677,"context_line":"                host\u003dhost,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_9ca5e0a1","line":1674,"range":{"start_line":1674,"start_character":30,"end_line":1674,"end_character":37},"in_reply_to":"3fa7e38b_fc797481","updated":"2019-10-28 19:55:10.000000000","message":"We should use it as passed in for sure. This patch was just a WIP hack until I built the next patch on top of it and never really cleaned up some of the mess I made here.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"586a2b67425569620e76ff0df1ee439ff3ba4e27","unresolved":false,"context_lines":[{"line_number":1677,"context_line":"                host\u003dhost,"},{"line_number":1678,"context_line":"                image_ids\u003dimage_ids)"},{"line_number":1679,"context_line":"            for image_id, status in result.items():"},{"line_number":1680,"context_line":"                cached, existing, error \u003d stats[image]"},{"line_number":1681,"context_line":"                if status \u003d\u003d \u0027error\u0027:"},{"line_number":1682,"context_line":"                    failed_images[image_id] +\u003d 1"},{"line_number":1683,"context_line":"                    error +\u003d 1"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_0ecc4887","line":1680,"range":{"start_line":1680,"start_character":48,"end_line":1680,"end_character":53},"updated":"2019-10-29 17:49:24.000000000","message":"Aha yeah I see the bug now.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e2fa06dfb684316b81be6b29bf2204b031ac005b","unresolved":false,"context_lines":[{"line_number":1677,"context_line":"                host\u003dhost,"},{"line_number":1678,"context_line":"                image_ids\u003dimage_ids)"},{"line_number":1679,"context_line":"            for image_id, status in result.items():"},{"line_number":1680,"context_line":"                cached, existing, error \u003d stats[image]"},{"line_number":1681,"context_line":"                if status \u003d\u003d \u0027error\u0027:"},{"line_number":1682,"context_line":"                    failed_images[image_id] +\u003d 1"},{"line_number":1683,"context_line":"                    error +\u003d 1"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_1c7fb066","line":1680,"updated":"2019-10-28 19:50:24.000000000","message":"We don\u0027t care about \u0027unsupported\u0027 here as a result?","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"182273d746fd15148bee217fd09ead61b20c678b","unresolved":false,"context_lines":[{"line_number":1677,"context_line":"                host\u003dhost,"},{"line_number":1678,"context_line":"                image_ids\u003dimage_ids)"},{"line_number":1679,"context_line":"            for image_id, status in result.items():"},{"line_number":1680,"context_line":"                cached, existing, error \u003d stats[image]"},{"line_number":1681,"context_line":"                if status \u003d\u003d \u0027error\u0027:"},{"line_number":1682,"context_line":"                    failed_images[image_id] +\u003d 1"},{"line_number":1683,"context_line":"                    error +\u003d 1"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_5cbd28b5","line":1680,"in_reply_to":"3fa7e38b_1c7fb066","updated":"2019-10-28 19:55:10.000000000","message":"Sure, I can add.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e2fa06dfb684316b81be6b29bf2204b031ac005b","unresolved":false,"context_lines":[{"line_number":1693,"context_line":"                for host in hosts:"},{"line_number":1694,"context_line":"                    service \u003d objects.Service.get_by_compute_host(target_ctxt,"},{"line_number":1695,"context_line":"                                                                  host)"},{"line_number":1696,"context_line":"                    if not self.servicegroup_api.service_is_up(service):"},{"line_number":1697,"context_line":"                        LOG.info("},{"line_number":1698,"context_line":"                            \u0027Skipping image pre-cache request to compute \u0027"},{"line_number":1699,"context_line":"                            \u0027%(host)r because it is not up\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_fcd89452","line":1696,"updated":"2019-10-28 19:50:24.000000000","message":"You don\u0027t want to track anything for overall stats when we can\u0027t even communicate with a host? I mean, I know we already log something here, but if you\u0027re going for a kind of summary/report thing below, it seems we could count something for this as part of that summary.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"182273d746fd15148bee217fd09ead61b20c678b","unresolved":false,"context_lines":[{"line_number":1693,"context_line":"                for host in hosts:"},{"line_number":1694,"context_line":"                    service \u003d objects.Service.get_by_compute_host(target_ctxt,"},{"line_number":1695,"context_line":"                                                                  host)"},{"line_number":1696,"context_line":"                    if not self.servicegroup_api.service_is_up(service):"},{"line_number":1697,"context_line":"                        LOG.info("},{"line_number":1698,"context_line":"                            \u0027Skipping image pre-cache request to compute \u0027"},{"line_number":1699,"context_line":"                            \u0027%(host)r because it is not up\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_1cb330eb","line":1696,"in_reply_to":"3fa7e38b_fcd89452","updated":"2019-10-28 19:55:10.000000000","message":"Sure.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e2fa06dfb684316b81be6b29bf2204b031ac005b","unresolved":false,"context_lines":[{"line_number":1713,"context_line":"            overall_stats[\u0027error\u0027] +\u003d error"},{"line_number":1714,"context_line":""},{"line_number":1715,"context_line":"        clock.stop()"},{"line_number":1716,"context_line":"        LOG.info(\u0027Image pre-cache operation for image(s) %(image_ids)s \u0027"},{"line_number":1717,"context_line":"                 \u0027completed in %(time).2f seconds; \u0027"},{"line_number":1718,"context_line":"                 \u0027%(cached)i cached, %(existing)i existing, %(error)i errors\u0027,"},{"line_number":1719,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_5ca688c3","line":1716,"updated":"2019-10-28 19:50:24.000000000","message":"I was able to at least manually test this by changing the functional test nova.tests.functional.compute.test_cache_image.ImageCacheTest.test_cache_image to fail and it logged this:\n\n    b\u00272019-10-28 15:44:24,876 INFO [nova.conductor.manager] Preparing to request pre-caching of image(s) an-image on 4 hosts across 2 cells.\u0027\n    b\u00272019-10-28 15:44:24,912 INFO [nova.compute.manager] Caching 1 image(s) by request\u0027\n    b\u00272019-10-28 15:44:24,963 INFO [nova.compute.manager] Caching 1 image(s) by request\u0027\n    b\"2019-10-28 15:44:24,968 INFO [nova.conductor.manager] Skipping image pre-cache request to compute \u0027compute5\u0027 because it is not up\"\n    b\u00272019-10-28 15:44:24,973 INFO [nova.compute.manager] Caching 1 image(s) by request\u0027\n    b\u00272019-10-28 15:44:24,974 INFO [nova.conductor.manager] Image pre-cache operation for image(s) an-image completed in 0.10 seconds; 1 cached, 0 existing, 0 errors\u0027\n\nThat said, it doesn\u0027t account for anything in the \u0027existing\u0027 or \u0027error\u0027 cases and given the contract between the compute and conductor here with those hard-coded strings, it might not be terrible to add another functional test (or wrinkle in that existing functional test) that will hit the \u0027existing\u0027 and \u0027error\u0027 cases. I won\u0027t block on it though since this is pretty benign and don\u0027t want to push too much on over-complicating this.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"79f09de64b04b74e7783e64c01b17ef126568506","unresolved":false,"context_lines":[{"line_number":1713,"context_line":"            overall_stats[\u0027error\u0027] +\u003d error"},{"line_number":1714,"context_line":""},{"line_number":1715,"context_line":"        clock.stop()"},{"line_number":1716,"context_line":"        LOG.info(\u0027Image pre-cache operation for image(s) %(image_ids)s \u0027"},{"line_number":1717,"context_line":"                 \u0027completed in %(time).2f seconds; \u0027"},{"line_number":1718,"context_line":"                 \u0027%(cached)i cached, %(existing)i existing, %(error)i errors\u0027,"},{"line_number":1719,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_d20a5f24","line":1716,"in_reply_to":"3fa7e38b_5ca688c3","updated":"2019-10-28 21:10:31.000000000","message":"Actually, it was wrong. Your line that says \"1 cached\" should have been \"3 cached\". Test written, bug fixed :)","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e2fa06dfb684316b81be6b29bf2204b031ac005b","unresolved":false,"context_lines":[{"line_number":1723,"context_line":"                  \u0027error\u0027: overall_stats[\u0027error\u0027],"},{"line_number":1724,"context_line":"                 })"},{"line_number":1725,"context_line":"        # Log error\u0027d images specifically at warning level"},{"line_number":1726,"context_line":"        for image_id, fails in failed_images.items():"},{"line_number":1727,"context_line":"            LOG.warning(\u0027Image pre-cache operation for image %(image)s \u0027"},{"line_number":1728,"context_line":"                        \u0027failed %(fails)i times\u0027,"},{"line_number":1729,"context_line":"                        {\u0027image\u0027: image_id,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_5c6528b4","line":1726,"updated":"2019-10-28 19:50:24.000000000","message":"\u003e I know ya\u0027ll\u0027re going to want log tests for this, but I wanted to get the next one up for comments and this was in the way.\n\nSeems like a simple unit test for this logic would be useful. It\u0027s not like there is no value in unit testing this because I\u0027ve (1) seen bugs in log messages before, e.g. bad variable substitution and (2) it could help prevent regressions if this code is refactored in the future.","commit_id":"ba9db06735917fa6a6aa90868df4de7a92945b71"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"586a2b67425569620e76ff0df1ee439ff3ba4e27","unresolved":false,"context_lines":[{"line_number":3665,"context_line":"    @mock.patch(\u0027nova.objects.HostMapping.get_by_host\u0027)"},{"line_number":3666,"context_line":"    @mock.patch(\u0027nova.context.target_cell\u0027)"},{"line_number":3667,"context_line":"    @mock.patch(\u0027nova.objects.Service.get_by_compute_host\u0027)"},{"line_number":3668,"context_line":"    def test_cache_images_failed_compute(self, mock_service, mock_target,"},{"line_number":3669,"context_line":"                                         mock_gbh):"},{"line_number":3670,"context_line":"        \"\"\"Test the edge cases for cache_images(), specifically the"},{"line_number":3671,"context_line":"        error, skip, and down situations."}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_aec0942c","line":3668,"range":{"start_line":3668,"start_character":8,"end_line":3668,"end_character":40},"updated":"2019-10-29 17:49:24.000000000","message":"Would be nice to move this above test_cache_images_missing because it\u0027s going to conflict with a good chunk of my cross-cell series which adds new conductor methods for resize confirm/revert. Not a huge deal since I\u0027ll likely have to rebase that thing soon anyway.","commit_id":"7ecd502f6df01efe8ed829cd0513df1d7bf9f9da"}]}