)]}'
{"nova/conductor/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":159,"context_line":"                host\u003dhost,"},{"line_number":160,"context_line":"                request_spec\u003drequest_spec)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def cache_images(self, context, aggregate, image_ids):"},{"line_number":163,"context_line":"        for image_id in image_ids:"},{"line_number":164,"context_line":"            try:"},{"line_number":165,"context_line":"                self.image_api.get(context, image_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_118a4b69","line":162,"range":{"start_line":162,"start_character":8,"end_line":162,"end_character":20},"updated":"2019-10-09 19:57:54.000000000","message":"docstring would be nice","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":162,"context_line":"    def cache_images(self, context, aggregate, image_ids):"},{"line_number":163,"context_line":"        for image_id in image_ids:"},{"line_number":164,"context_line":"            try:"},{"line_number":165,"context_line":"                self.image_api.get(context, image_id)"},{"line_number":166,"context_line":"            except exception.NovaException:"},{"line_number":167,"context_line":"                raise"},{"line_number":168,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_d1cd93b7","line":165,"updated":"2019-10-09 19:57:54.000000000","message":"Might want a comment about why we\u0027re getting the images here, I guess this is just validation? I would expect this to happen in the compute API so we can fail fast before returning the 202 response.\n\n--\n\nAlso, rather than loop and GET each image, it looks like we could get them all in a single query to the image API:\n\nhttps://docs.openstack.org/api-ref/image/v2/index.html?expanded\u003dlist-images-detail#list-images\n\ne.g.:\n\nTo find images in a particular list of image IDs, use:\n\nGET /v2/images?id\u003din:3afb79c1-131a-4c38-a87c-bc4b801d14e6,2e011209-660f-44b5-baf2-2eb4babae53d\n\nThe only problem is if that doesn\u0027t 404 if one of the images is not found since it\u0027s an IN type operation. So I guess we can\u0027t do that. Nevermind.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"113db952285d1aa7b42566affdbabd978b9403df","unresolved":false,"context_lines":[{"line_number":162,"context_line":"    def cache_images(self, context, aggregate, image_ids):"},{"line_number":163,"context_line":"        for image_id in image_ids:"},{"line_number":164,"context_line":"            try:"},{"line_number":165,"context_line":"                self.image_api.get(context, image_id)"},{"line_number":166,"context_line":"            except exception.NovaException:"},{"line_number":167,"context_line":"                raise"},{"line_number":168,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_3ba70fbf","line":165,"in_reply_to":"3fa7e38b_30117305","updated":"2019-10-10 19:59:28.000000000","message":"Yeah I meant REST API i.e. wsgi route handler or nova.compute.api.API (before I saw that you didn\u0027t touch nova.compute.api.API and just handle calling conductor from the route handler). And yeah this is still synchronous and will affect the response code, it just seemed a bit detached from normal procedure which is to do validation in one of those other places.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":162,"context_line":"    def cache_images(self, context, aggregate, image_ids):"},{"line_number":163,"context_line":"        for image_id in image_ids:"},{"line_number":164,"context_line":"            try:"},{"line_number":165,"context_line":"                self.image_api.get(context, image_id)"},{"line_number":166,"context_line":"            except exception.NovaException:"},{"line_number":167,"context_line":"                raise"},{"line_number":168,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_30117305","line":165,"in_reply_to":"3fa7e38b_d1cd93b7","updated":"2019-10-10 14:35:46.000000000","message":"\u003e Might want a comment about why we\u0027re getting the images here, I\n \u003e guess this is just validation? I would expect this to happen in the\n \u003e compute API so we can fail fast before returning the 202 response.\n\nBy \"compute API\" do you mean the REST API? Since this is in conductor/api we\u0027re calling this synchronously and it does affect the 202 return if it fails. It breaks free when we make the rpc cast below.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            except exception.NovaException:"},{"line_number":167,"context_line":"                raise"},{"line_number":168,"context_line":"            except Exception:"},{"line_number":169,"context_line":"                raise exception.ImageUnacceptable(image_id)"},{"line_number":170,"context_line":"        self.conductor_compute_rpcapi.cache_images(context, aggregate,"},{"line_number":171,"context_line":"                                                   image_ids)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_318e6779","line":169,"range":{"start_line":169,"start_character":50,"end_line":169,"end_character":58},"updated":"2019-10-09 19:57:54.000000000","message":"This isn\u0027t enough, the exception takes 2 kwargs: image_id and reason. You\u0027re overwriting the exception message with just the image ID so you\u0027ll lose the actual reason for the failure. Should be something like:\n\nexcept Exception as e:\n    raise exception.ImageUnacceptable(image_id\u003dimage_id,\n                                      reason\u003dsix.text_type(e))\n\n-1 for this.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"}],"nova/conductor/manager.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"0174b77858f2a83e7a9ac2001c0b90c537ce9c2e","unresolved":false,"context_lines":[{"line_number":1659,"context_line":"            cell \u003d cells_by_uuid[cell_uuid]"},{"line_number":1660,"context_line":"            with nova_context.target_cell(context, cell) as target_ctxt:"},{"line_number":1661,"context_line":"                for host in hosts:"},{"line_number":1662,"context_line":"                    fetch_pool.spawn_n(self.compute_rpcapi.cache_images,"},{"line_number":1663,"context_line":"                                       target_ctxt,"},{"line_number":1664,"context_line":"                                       host\u003dhost,"},{"line_number":1665,"context_line":"                                       image_ids\u003dimage_ids)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_a2222400","line":1662,"updated":"2019-10-08 15:50:08.000000000","message":"The NovaException that this can raise will get bubbled up through here, right? (I\u0027m not sure how Exceptions are handled with thread pools). So if this explodes with a NovaException... I mean it\u0027s not good, but what exactly are the specific bad parts? We never call waitall(), clock.stop()...?","commit_id":"a62d5291077ed59c44cc461a3480c45b07ef537b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c2f94b075ea6bfc8edb8f0c07510e3c5389bc188","unresolved":false,"context_lines":[{"line_number":1659,"context_line":"            cell \u003d cells_by_uuid[cell_uuid]"},{"line_number":1660,"context_line":"            with nova_context.target_cell(context, cell) as target_ctxt:"},{"line_number":1661,"context_line":"                for host in hosts:"},{"line_number":1662,"context_line":"                    fetch_pool.spawn_n(self.compute_rpcapi.cache_images,"},{"line_number":1663,"context_line":"                                       target_ctxt,"},{"line_number":1664,"context_line":"                                       host\u003dhost,"},{"line_number":1665,"context_line":"                                       image_ids\u003dimage_ids)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_4576da08","line":1662,"in_reply_to":"3fa7e38b_a2222400","updated":"2019-10-08 16:27:40.000000000","message":"You mean the *compute* rpc fail? Yeah, it will raise and kill the call but not us. So we\u0027ll go through them all, they will fail and we should still call waitall() below.\n\nIt\u0027ll probably generate an error message for each one, which may or may not be desired, I dunno.","commit_id":"a62d5291077ed59c44cc461a3480c45b07ef537b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":1632,"context_line":"        return True"},{"line_number":1633,"context_line":""},{"line_number":1634,"context_line":"    def cache_images(self, context, aggregate, image_ids):"},{"line_number":1635,"context_line":"        \"\"\"Cache an set of images on the set of hosts in an aggregate."},{"line_number":1636,"context_line":""},{"line_number":1637,"context_line":"        :param context: The RequestContext"},{"line_number":1638,"context_line":"        :param aggregate: The Aggregate object from the request to constrain"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_91dabb75","line":1635,"range":{"start_line":1635,"start_character":17,"end_line":1635,"end_character":19},"updated":"2019-10-09 19:57:54.000000000","message":"a?","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":1647,"context_line":""},{"line_number":1648,"context_line":"        clock \u003d timeutils.StopWatch()"},{"line_number":1649,"context_line":"        threads \u003d CONF.image_cache.precache_parallelism"},{"line_number":1650,"context_line":"        fetch_pool \u003d eventlet.GreenPool(size\u003dthreads)"},{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"        hosts_by_cell \u003d {}"},{"line_number":1653,"context_line":"        cells_by_uuid \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_943949d8","line":1650,"updated":"2019-10-09 19:57:54.000000000","message":"Any thoughts on using a ThreadPoolExecutor here rather than eventlet?\n\nhttps://docs.python.org/3.6/library/concurrent.futures.html#threadpoolexecutor\n\nI would think that would be a nice way to get the results back from a Future that is doing the RPC call to the compute. Though if/when you want the results you\u0027d just change to use spawn rather than spawn_n.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":1647,"context_line":""},{"line_number":1648,"context_line":"        clock \u003d timeutils.StopWatch()"},{"line_number":1649,"context_line":"        threads \u003d CONF.image_cache.precache_parallelism"},{"line_number":1650,"context_line":"        fetch_pool \u003d eventlet.GreenPool(size\u003dthreads)"},{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"        hosts_by_cell \u003d {}"},{"line_number":1653,"context_line":"        cells_by_uuid \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f01f5b05","line":1650,"in_reply_to":"3fa7e38b_943949d8","updated":"2019-10-10 14:35:46.000000000","message":"I\u0027m doing this to be in-line with how compute manager currently handles pools of threads, so I\u0027d prefer to keep this the same or switch it all out.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":1649,"context_line":"        threads \u003d CONF.image_cache.precache_parallelism"},{"line_number":1650,"context_line":"        fetch_pool \u003d eventlet.GreenPool(size\u003dthreads)"},{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"        hosts_by_cell \u003d {}"},{"line_number":1653,"context_line":"        cells_by_uuid \u003d {}"},{"line_number":1654,"context_line":"        # TODO(danms): Make this much a more efficient bulk query"},{"line_number":1655,"context_line":"        for hostname in aggregate.hosts:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_b1cd7731","line":1652,"range":{"start_line":1652,"start_character":24,"end_line":1652,"end_character":26},"updated":"2019-10-09 19:57:54.000000000","message":"nit: could be collections.defaultdict(list)","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"        hosts_by_cell \u003d {}"},{"line_number":1653,"context_line":"        cells_by_uuid \u003d {}"},{"line_number":1654,"context_line":"        # TODO(danms): Make this much a more efficient bulk query"},{"line_number":1655,"context_line":"        for hostname in aggregate.hosts:"},{"line_number":1656,"context_line":"            hmap \u003d objects.HostMapping.get_by_host(context, hostname)"},{"line_number":1657,"context_line":"            cells_by_uuid.setdefault(hmap.cell_mapping.uuid, hmap.cell_mapping)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f1c76f11","line":1654,"range":{"start_line":1654,"start_character":33,"end_line":1654,"end_character":44},"updated":"2019-10-09 19:57:54.000000000","message":"a much more","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"        hosts_by_cell \u003d {}"},{"line_number":1653,"context_line":"        cells_by_uuid \u003d {}"},{"line_number":1654,"context_line":"        # TODO(danms): Make this much a more efficient bulk query"},{"line_number":1655,"context_line":"        for hostname in aggregate.hosts:"},{"line_number":1656,"context_line":"            hmap \u003d objects.HostMapping.get_by_host(context, hostname)"},{"line_number":1657,"context_line":"            cells_by_uuid.setdefault(hmap.cell_mapping.uuid, hmap.cell_mapping)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_30df333b","line":1654,"range":{"start_line":1654,"start_character":33,"end_line":1654,"end_character":44},"in_reply_to":"3fa7e38b_f1c76f11","updated":"2019-10-10 14:35:46.000000000","message":"No \"that\u0027s much amore\" joke? Come on.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":1678,"context_line":"                            {\u0027host\u0027: host})"},{"line_number":1679,"context_line":"                        continue"},{"line_number":1680,"context_line":""},{"line_number":1681,"context_line":"                    fetch_pool.spawn_n(self.compute_rpcapi.cache_images,"},{"line_number":1682,"context_line":"                                       target_ctxt,"},{"line_number":1683,"context_line":"                                       host\u003dhost,"},{"line_number":1684,"context_line":"                                       image_ids\u003dimage_ids)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_1137ab01","line":1681,"range":{"start_line":1681,"start_character":59,"end_line":1681,"end_character":71},"updated":"2019-10-09 19:57:54.000000000","message":"OK so this is a blocking call:\n\nhttps://review.opendev.org/#/c/687138/3/nova/compute/rpcapi.py\n\nThat returns a dict of results with specific values but they aren\u0027t being gathered here for anything. Were there plans on using those later for sending some per-host notification with the results?","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":1678,"context_line":"                            {\u0027host\u0027: host})"},{"line_number":1679,"context_line":"                        continue"},{"line_number":1680,"context_line":""},{"line_number":1681,"context_line":"                    fetch_pool.spawn_n(self.compute_rpcapi.cache_images,"},{"line_number":1682,"context_line":"                                       target_ctxt,"},{"line_number":1683,"context_line":"                                       host\u003dhost,"},{"line_number":1684,"context_line":"                                       image_ids\u003dimage_ids)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_d0d01f22","line":1681,"range":{"start_line":1681,"start_character":59,"end_line":1681,"end_character":71},"in_reply_to":"3fa7e38b_1137ab01","updated":"2019-10-10 14:35:46.000000000","message":"I was thinking we\u0027d just collect some stats in a follow on patch to include in the log below. I was also expecting the computes to send their own notifications, but we\u0027ve punted on that per our discussion until this part is done. So, if we decide to do it here, then sure, we\u0027d use that, otherwise I\u0027ll just add up some \"N done, N existing, N failed\" log traffic, maybe with \"Hmm, image X failed everywhere\" stats.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":1688,"context_line":""},{"line_number":1689,"context_line":"        clock.stop()"},{"line_number":1690,"context_line":"        LOG.info(\u0027Image pre-cache operation for image(s) %(image_ids)s \u0027"},{"line_number":1691,"context_line":"                 \u0027completed in %(time)i seconds\u0027,"},{"line_number":1692,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"},{"line_number":1693,"context_line":"                  \u0027time\u0027: clock.elapsed()})"},{"line_number":1694,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_71893fc8","line":1691,"range":{"start_line":1691,"start_character":31,"end_line":1691,"end_character":39},"updated":"2019-10-09 19:57:54.000000000","message":"Format this to 2 decimal points?","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"29b42f1c4f7f0adccccdacf8fa8e5fa26e814e4c","unresolved":false,"context_lines":[{"line_number":1688,"context_line":""},{"line_number":1689,"context_line":"        clock.stop()"},{"line_number":1690,"context_line":"        LOG.info(\u0027Image pre-cache operation for image(s) %(image_ids)s \u0027"},{"line_number":1691,"context_line":"                 \u0027completed in %(time)i seconds\u0027,"},{"line_number":1692,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"},{"line_number":1693,"context_line":"                  \u0027time\u0027: clock.elapsed()})"},{"line_number":1694,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_89492fb8","line":1691,"range":{"start_line":1691,"start_character":31,"end_line":1691,"end_character":39},"in_reply_to":"3fa7e38b_71893fc8","updated":"2019-10-10 07:13:46.000000000","message":"2 decimal points maybe better. In the case of higher bandwidth, if the image is small, it may be completed in one second.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":1688,"context_line":""},{"line_number":1689,"context_line":"        clock.stop()"},{"line_number":1690,"context_line":"        LOG.info(\u0027Image pre-cache operation for image(s) %(image_ids)s \u0027"},{"line_number":1691,"context_line":"                 \u0027completed in %(time)i seconds\u0027,"},{"line_number":1692,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"},{"line_number":1693,"context_line":"                  \u0027time\u0027: clock.elapsed()})"},{"line_number":1694,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_909b8771","line":1691,"range":{"start_line":1691,"start_character":31,"end_line":1691,"end_character":39},"in_reply_to":"3fa7e38b_89492fb8","updated":"2019-10-10 14:35:46.000000000","message":"Meh, fine.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99b04dee01e5c5b88938ffda03ea21b9aa4672a8","unresolved":false,"context_lines":[{"line_number":1640,"context_line":"        :param image_id: The IDs of the image to cache"},{"line_number":1641,"context_line":"        \"\"\""},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"        # TODO(danms): Fix notification sample for IMAGE_CACHE action"},{"line_number":1644,"context_line":"        compute_utils.notify_about_aggregate_action("},{"line_number":1645,"context_line":"            context, aggregate,"},{"line_number":1646,"context_line":"            fields.NotificationAction.IMAGE_CACHE,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_84e257d4","line":1643,"updated":"2019-10-11 15:52:17.000000000","message":"I guess this is here not to forget to add a notification sample test.","commit_id":"0e7fe0c85be307c45f2c2dee66517096f56c9e47"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c5e4dbf1cae2dfb819b3fe46ed3bff5428fd04d5","unresolved":false,"context_lines":[{"line_number":1640,"context_line":"        :param image_id: The IDs of the image to cache"},{"line_number":1641,"context_line":"        \"\"\""},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"        # TODO(danms): Fix notification sample for IMAGE_CACHE action"},{"line_number":1644,"context_line":"        compute_utils.notify_about_aggregate_action("},{"line_number":1645,"context_line":"            context, aggregate,"},{"line_number":1646,"context_line":"            fields.NotificationAction.IMAGE_CACHE,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_6495dbe3","line":1643,"in_reply_to":"3fa7e38b_84e257d4","updated":"2019-10-11 16:01:10.000000000","message":"Yeah, per mriedem. After this series, we\u0027ll be adding a new notification per compute so that someone that wants to will be able to watch notifications and determine which hosts are pending/complete. He wasn\u0027t sure the magic needed for that (and I definitely don\u0027t know) so we were going to get your help with that, and that\u0027s why this TODO is here, so we can chuck that in that patch.","commit_id":"0e7fe0c85be307c45f2c2dee66517096f56c9e47"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99b04dee01e5c5b88938ffda03ea21b9aa4672a8","unresolved":false,"context_lines":[{"line_number":1660,"context_line":"            hosts_by_cell[hmap.cell_mapping.uuid].append(hostname)"},{"line_number":1661,"context_line":""},{"line_number":1662,"context_line":"        LOG.info(\u0027Preparing to request pre-caching of image(s) %(image_ids)s \u0027"},{"line_number":1663,"context_line":"                 \u0027on %(hosts)i hosts across %(cells)s cells.\u0027,"},{"line_number":1664,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"},{"line_number":1665,"context_line":"                  \u0027hosts\u0027: len(aggregate.hosts),"},{"line_number":1666,"context_line":"                  \u0027cells\u0027: len(hosts_by_cell)})"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_e4dc6b75","line":1663,"range":{"start_line":1663,"start_character":21,"end_line":1663,"end_character":53},"updated":"2019-10-11 15:52:17.000000000","message":"Both cells and hosts are integers, why they are formatted differently?","commit_id":"0e7fe0c85be307c45f2c2dee66517096f56c9e47"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c5e4dbf1cae2dfb819b3fe46ed3bff5428fd04d5","unresolved":false,"context_lines":[{"line_number":1660,"context_line":"            hosts_by_cell[hmap.cell_mapping.uuid].append(hostname)"},{"line_number":1661,"context_line":""},{"line_number":1662,"context_line":"        LOG.info(\u0027Preparing to request pre-caching of image(s) %(image_ids)s \u0027"},{"line_number":1663,"context_line":"                 \u0027on %(hosts)i hosts across %(cells)s cells.\u0027,"},{"line_number":1664,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"},{"line_number":1665,"context_line":"                  \u0027hosts\u0027: len(aggregate.hosts),"},{"line_number":1666,"context_line":"                  \u0027cells\u0027: len(hosts_by_cell)})"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_04018738","line":1663,"range":{"start_line":1663,"start_character":21,"end_line":1663,"end_character":53},"in_reply_to":"3fa7e38b_e4dc6b75","updated":"2019-10-11 16:01:10.000000000","message":"Because I originally was reporting this differently and didn\u0027t change it. I\u0027ll fix.","commit_id":"0e7fe0c85be307c45f2c2dee66517096f56c9e47"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99b04dee01e5c5b88938ffda03ea21b9aa4672a8","unresolved":false,"context_lines":[{"line_number":1691,"context_line":"        LOG.info(\u0027Image pre-cache operation for image(s) %(image_ids)s \u0027"},{"line_number":1692,"context_line":"                 \u0027completed in %(time).2f seconds\u0027,"},{"line_number":1693,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"},{"line_number":1694,"context_line":"                  \u0027time\u0027: clock.elapsed()})"},{"line_number":1695,"context_line":""},{"line_number":1696,"context_line":"        compute_utils.notify_about_aggregate_action("},{"line_number":1697,"context_line":"            context, aggregate,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_c4660fbc","line":1694,"updated":"2019-10-11 15:52:17.000000000","message":"You wrote code to compile a dict with the result of the caching on the compute side. But it is not used here and not even printed on the compute side.\n\nAlso if we don\u0027t care about the result then we could change compute_rpcapi.cache_images an rpc cast instead of a call.","commit_id":"0e7fe0c85be307c45f2c2dee66517096f56c9e47"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c5e4dbf1cae2dfb819b3fe46ed3bff5428fd04d5","unresolved":false,"context_lines":[{"line_number":1691,"context_line":"        LOG.info(\u0027Image pre-cache operation for image(s) %(image_ids)s \u0027"},{"line_number":1692,"context_line":"                 \u0027completed in %(time).2f seconds\u0027,"},{"line_number":1693,"context_line":"                 {\u0027image_ids\u0027: \u0027,\u0027.join(image_ids),"},{"line_number":1694,"context_line":"                  \u0027time\u0027: clock.elapsed()})"},{"line_number":1695,"context_line":""},{"line_number":1696,"context_line":"        compute_utils.notify_about_aggregate_action("},{"line_number":1697,"context_line":"            context, aggregate,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_048667ba","line":1694,"in_reply_to":"3fa7e38b_c4660fbc","updated":"2019-10-11 16:01:10.000000000","message":"No, we really need it to be a call because we need to throttle this to the concurrency limit (per the spec). As we just discussed on IRC, I am planning a follow-on patch to compute some stats to log here, which will use the result dict, but wanted to separate that from the base functionality added here.","commit_id":"0e7fe0c85be307c45f2c2dee66517096f56c9e47"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a01efbe2aa99dd3903cee1251e4d0a231b7ae8a","unresolved":false,"context_lines":[{"line_number":1641,"context_line":"        \"\"\""},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"        # TODO(danms): Fix notification sample for IMAGE_CACHE action"},{"line_number":1644,"context_line":"        compute_utils.notify_about_aggregate_action("},{"line_number":1645,"context_line":"            context, aggregate,"},{"line_number":1646,"context_line":"            fields.NotificationAction.IMAGE_CACHE,"},{"line_number":1647,"context_line":"            fields.NotificationPhase.START)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_1c86f766","line":1644,"updated":"2019-10-24 22:04:02.000000000","message":"I realize a bit late now that this won\u0027t have the image_ids in it but that would have probably been good to add. We could add it with a version bump on the payload though.","commit_id":"11d909c2cbf80ce899b7a68ce72897dfb2945fed"}],"nova/conductor/rpcapi.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"0174b77858f2a83e7a9ac2001c0b90c537ce9c2e","unresolved":false,"context_lines":[{"line_number":442,"context_line":"    def cache_images(self, ctxt, aggregate, image_ids):"},{"line_number":443,"context_line":"        version \u003d \u00271.21\u0027"},{"line_number":444,"context_line":"        if not self.client.can_send_version(version):"},{"line_number":445,"context_line":"            raise exception.NovaException(\u0027Conductor RPC version pin does not \u0027"},{"line_number":446,"context_line":"                                          \u0027allow cache_image() to be called\u0027)"},{"line_number":447,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":448,"context_line":"        cctxt.cast(ctxt, \u0027cache_images\u0027, aggregate\u003daggregate,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_028a181a","line":445,"updated":"2019-10-08 15:50:08.000000000","message":"This\u0027ll get caught by the API layer in the patch on top of this one.","commit_id":"a62d5291077ed59c44cc461a3480c45b07ef537b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"0174b77858f2a83e7a9ac2001c0b90c537ce9c2e","unresolved":false,"context_lines":[{"line_number":445,"context_line":"            raise exception.NovaException(\u0027Conductor RPC version pin does not \u0027"},{"line_number":446,"context_line":"                                          \u0027allow cache_image() to be called\u0027)"},{"line_number":447,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":448,"context_line":"        cctxt.cast(ctxt, \u0027cache_images\u0027, aggregate\u003daggregate,"},{"line_number":449,"context_line":"                   image_ids\u003dimage_ids)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_228dd430","line":448,"updated":"2019-10-08 15:50:08.000000000","message":"But this is a cast, so any exception \"below\" this (like that NovaException) will... just get logged?","commit_id":"a62d5291077ed59c44cc461a3480c45b07ef537b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c2f94b075ea6bfc8edb8f0c07510e3c5389bc188","unresolved":false,"context_lines":[{"line_number":445,"context_line":"            raise exception.NovaException(\u0027Conductor RPC version pin does not \u0027"},{"line_number":446,"context_line":"                                          \u0027allow cache_image() to be called\u0027)"},{"line_number":447,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":448,"context_line":"        cctxt.cast(ctxt, \u0027cache_images\u0027, aggregate\u003daggregate,"},{"line_number":449,"context_line":"                   image_ids\u003dimage_ids)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c5d08acc","line":448,"in_reply_to":"3fa7e38b_228dd430","updated":"2019-10-08 16:27:40.000000000","message":"Yep, on the conductor worker.","commit_id":"a62d5291077ed59c44cc461a3480c45b07ef537b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":443,"context_line":"        version \u003d \u00271.21\u0027"},{"line_number":444,"context_line":"        if not self.client.can_send_version(version):"},{"line_number":445,"context_line":"            raise exception.NovaException(\u0027Conductor RPC version pin does not \u0027"},{"line_number":446,"context_line":"                                          \u0027allow cache_image() to be called\u0027)"},{"line_number":447,"context_line":"        cctxt \u003d self.client.prepare(version\u003dversion)"},{"line_number":448,"context_line":"        cctxt.cast(ctxt, \u0027cache_images\u0027, aggregate\u003daggregate,"},{"line_number":449,"context_line":"                   image_ids\u003dimage_ids)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_71c61f9d","line":446,"range":{"start_line":446,"start_character":49,"end_line":446,"end_character":60},"updated":"2019-10-09 19:57:54.000000000","message":"cache_images","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"}],"nova/conf/imagecache.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":16,"context_line":"    \u0027image_cache\u0027,"},{"line_number":17,"context_line":"    title\u003d\u0027Image Cache Options\u0027,"},{"line_number":18,"context_line":"    help\u003d\"\"\""},{"line_number":19,"context_line":"A collection of options specific to image caching."},{"line_number":20,"context_line":"\"\"\")"},{"line_number":21,"context_line":"imagecache_opts \u003d ["},{"line_number":22,"context_line":"    cfg.IntOpt(\u0027precache_parallelism\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_3160a7cf","line":19,"updated":"2019-10-09 19:57:54.000000000","message":"This doesn\u0027t show up for some reason:\n\nhttps://517a43f21d8cde229b6a-f833862f6fa359097814ed1d3103e043.ssl.cf5.rackcdn.com/687139/4/check/openstack-tox-docs/ff36c38/docs/configuration/config.html#image_cache\n\nand I think I know why, see below.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def list_opts():"},{"line_number":47,"context_line":"    return {\u0027image_cache\u0027: imagecache_opts}"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_b1a1f7f9","line":47,"range":{"start_line":47,"start_character":12,"end_line":47,"end_character":25},"updated":"2019-10-09 19:57:54.000000000","message":"I think to get the group help rendered, you need to use the OptGroup object here (I remember hitting this same issue in Watcher).\n\nThat\u0027s also how the upgrade_levels group stuff works and the group help for that works properly.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d813259eaca11b3b7fd50c337a37f2ff1baad09f","unresolved":false,"context_lines":[{"line_number":19,"context_line":"A collection of options specific to image caching."},{"line_number":20,"context_line":"\"\"\")"},{"line_number":21,"context_line":"imagecache_opts \u003d ["},{"line_number":22,"context_line":"    cfg.IntOpt(\u0027precache_parallelism\u0027,"},{"line_number":23,"context_line":"               default\u003d1,"},{"line_number":24,"context_line":"               min\u003d1,"},{"line_number":25,"context_line":"               help\u003d\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_fbcbb74f","line":22,"range":{"start_line":22,"start_character":25,"end_line":22,"end_character":36},"updated":"2019-10-10 20:06:15.000000000","message":"nit: as mentioned in the API change that adds the release note, I would go with \"concurrency\" here but that\u0027s just me.","commit_id":"073da99d0cfbf5a50aa8a6f019c17ef477081526"}],"nova/notifications/objects/base.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    #               NotificationActionField enum"},{"line_number":72,"context_line":"    # Version 1.19: SELECT_DESTINATIONS is added to the NotificationActionField"},{"line_number":73,"context_line":"    #               enum"},{"line_number":74,"context_line":"    # Version 1.20: IMAGE_CACHE is added to the NotificationActionField enum"},{"line_number":75,"context_line":"    VERSION \u003d \u00271.20\u0027"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f14c4f9f","line":74,"updated":"2019-10-09 19:57:54.000000000","message":"We should get a sample for this:\n\nhttps://docs.openstack.org/nova/latest/reference/notifications.html#versioned-notification-samples\n\nI think that would be generated from a new test here:\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/functional/notification_sample_tests/test_aggregate.py\n\nIf you want to defer that to a separate change I\u0027m OK with it since I\u0027m not sure how tricky that is, but we should leave a TODO for doing it somewhere (maybe in the conductor compute task manager method that is issuing the new notification).","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"}],"nova/objects/fields.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":864,"context_line":"    BUILD_INSTANCES \u003d \u0027build_instances\u0027"},{"line_number":865,"context_line":"    MIGRATE_SERVER \u003d \u0027migrate_server\u0027"},{"line_number":866,"context_line":"    REBUILD_SERVER \u003d \u0027rebuild_server\u0027"},{"line_number":867,"context_line":"    IMAGE_CACHE \u003d \u0027image_cache\u0027"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    ALL \u003d (UPDATE, EXCEPTION, DELETE, PAUSE, UNPAUSE, RESIZE, VOLUME_SWAP,"},{"line_number":870,"context_line":"           SUSPEND, POWER_ON, REBOOT, SHUTDOWN, SNAPSHOT, INTERFACE_ATTACH,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_91539b6b","line":867,"range":{"start_line":867,"start_character":19,"end_line":867,"end_character":30},"updated":"2019-10-09 19:57:54.000000000","message":"nit: would \"cache_images\" be better here to match the conductor methods? Not a big deal though.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":864,"context_line":"    BUILD_INSTANCES \u003d \u0027build_instances\u0027"},{"line_number":865,"context_line":"    MIGRATE_SERVER \u003d \u0027migrate_server\u0027"},{"line_number":866,"context_line":"    REBUILD_SERVER \u003d \u0027rebuild_server\u0027"},{"line_number":867,"context_line":"    IMAGE_CACHE \u003d \u0027image_cache\u0027"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    ALL \u003d (UPDATE, EXCEPTION, DELETE, PAUSE, UNPAUSE, RESIZE, VOLUME_SWAP,"},{"line_number":870,"context_line":"           SUSPEND, POWER_ON, REBOOT, SHUTDOWN, SNAPSHOT, INTERFACE_ATTACH,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_1096b73f","line":867,"range":{"start_line":867,"start_character":19,"end_line":867,"end_character":30},"in_reply_to":"3fa7e38b_91539b6b","updated":"2019-10-10 14:35:46.000000000","message":"Goes with the other \"verb_noun\" examples here at least.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"}],"nova/tests/functional/compute/test_cache_image.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    def setUp(self):"},{"line_number":26,"context_line":"        super(ImageCacheTest, self).setUp()"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"        fake_notifier.stub_notifier(self)"},{"line_number":29,"context_line":"        self.context \u003d context.get_admin_context()"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"        self.conductor \u003d self.start_service(\u0027conductor\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f49bddc5","line":28,"range":{"start_line":28,"start_character":22,"end_line":28,"end_character":35},"updated":"2019-10-09 19:57:54.000000000","message":"You should reset this:\n\nself.addCleanup(fake_notifier.reset)","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        self.start_service(\u0027compute\u0027, host\u003d\u0027compute4\u0027, cell\u003d\u0027cell2\u0027)"},{"line_number":36,"context_line":"        self.start_service(\u0027compute\u0027, host\u003d\u0027compute5\u0027, cell\u003d\u0027cell2\u0027)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        srv \u003d objects.Service.get_by_compute_host(self.context, \u0027compute5\u0027)"},{"line_number":39,"context_line":"        srv.forced_down \u003d True"},{"line_number":40,"context_line":"        srv.save()"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_348e5579","line":38,"range":{"start_line":38,"start_character":8,"end_line":38,"end_character":75},"updated":"2019-10-09 19:57:54.000000000","message":"I\u0027m surprised this works because you created compute5 in cell2 but the context is not targeted to cell2, so wouldn\u0027t it by default target to cell1 because of the CellDatabases fixture?\n\nhttps://github.com/openstack/nova/blob/2565cd841d56beee15d85545d7df509e45b0713c/nova/test.py#L373","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c70c16acb6bbf25ec15f25fbfd6e04a077e33d42","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        self.start_service(\u0027compute\u0027, host\u003d\u0027compute4\u0027, cell\u003d\u0027cell2\u0027)"},{"line_number":36,"context_line":"        self.start_service(\u0027compute\u0027, host\u003d\u0027compute5\u0027, cell\u003d\u0027cell2\u0027)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        srv \u003d objects.Service.get_by_compute_host(self.context, \u0027compute5\u0027)"},{"line_number":39,"context_line":"        srv.forced_down \u003d True"},{"line_number":40,"context_line":"        srv.save()"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_c3a37b9d","line":38,"range":{"start_line":38,"start_character":8,"end_line":38,"end_character":75},"in_reply_to":"3fa7e38b_348e5579","updated":"2019-10-10 14:54:49.000000000","message":"Looks like stale residue because I just targeted cell2 when creating that service. I\u0027ll target for explicitness.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        srv.forced_down \u003d True"},{"line_number":40,"context_line":"        srv.save()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    @mock.patch(\u0027nova.virt.fake.FakeDriver.cache_image\u0027)"},{"line_number":43,"context_line":"    def test_cache_image(self, mock_cache):"},{"line_number":44,"context_line":"        \"\"\"Test caching images by injecting the request directly to"},{"line_number":45,"context_line":"        the conductor service and making sure it fans out and calls"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_b4a1e5f9","line":42,"updated":"2019-10-09 19:57:54.000000000","message":"How about adding a fake driver that implements the method so we can avoid mocks in here?","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d402c36042d7d32a581e5df95223bf045d03fa94","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        srv.forced_down \u003d True"},{"line_number":40,"context_line":"        srv.save()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    @mock.patch(\u0027nova.virt.fake.FakeDriver.cache_image\u0027)"},{"line_number":43,"context_line":"    def test_cache_image(self, mock_cache):"},{"line_number":44,"context_line":"        \"\"\"Test caching images by injecting the request directly to"},{"line_number":45,"context_line":"        the conductor service and making sure it fans out and calls"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_b4aec57e","line":42,"in_reply_to":"3fa7e38b_b4a1e5f9","updated":"2019-10-09 20:06:10.000000000","message":"You could use this right?\n\nhttps://review.opendev.org/#/c/687140/5/nova/virt/fake.py","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        srv.forced_down \u003d True"},{"line_number":40,"context_line":"        srv.save()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    @mock.patch(\u0027nova.virt.fake.FakeDriver.cache_image\u0027)"},{"line_number":43,"context_line":"    def test_cache_image(self, mock_cache):"},{"line_number":44,"context_line":"        \"\"\"Test caching images by injecting the request directly to"},{"line_number":45,"context_line":"        the conductor service and making sure it fans out and calls"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_d089ffc9","line":42,"in_reply_to":"3fa7e38b_b4aec57e","updated":"2019-10-10 14:35:46.000000000","message":"I could, but this is really an invasive functional test since we\u0027re calling straight into the conductor manager and bypassing all kinds of stuff. I really just wanted to test some of the plumbing here without too much fuss.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        cached \u003d []"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        def fake_cache_image(ctxt, image_id):"},{"line_number":51,"context_line":"            cached.append(image_id)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        mock_cache.side_effect \u003d fake_cache_image"},{"line_number":54,"context_line":"        aggregate \u003d objects.Aggregate(name\u003d\u0027test\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_b43d658f","line":51,"range":{"start_line":51,"start_character":12,"end_line":51,"end_character":35},"updated":"2019-10-09 19:57:54.000000000","message":"This might be a bit more interesting to assert if you stored the results per host e.g.:\n\ncached \u003d {}\n\ndef fake_cache_image(_self, ctxt, image_id):\n    cached[_self._host] \u003d image_id\n\nI\u0027m not really sure if that will work though.\n\nBut then below you could enhance what you have:\n\nself.assertEqual([\u0027an-image\u0027, \u0027an-image\u0027, \u0027an-image\u0027], cached)\n\nBut also asserting it\u0027s called on the computes you expect (1,3,4).","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8def89760e7d0fcaa65fac4f673e18655aa2194","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        cached \u003d []"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        def fake_cache_image(ctxt, image_id):"},{"line_number":51,"context_line":"            cached.append(image_id)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        mock_cache.side_effect \u003d fake_cache_image"},{"line_number":54,"context_line":"        aggregate \u003d objects.Aggregate(name\u003d\u0027test\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_305e5376","line":51,"range":{"start_line":51,"start_character":12,"end_line":51,"end_character":35},"in_reply_to":"3fa7e38b_b43d658f","updated":"2019-10-10 14:35:46.000000000","message":"Yeah I don\u0027t get self here which is why that\u0027s hard. I guess if I give into your request above I could. This is tested betterly in the later patch, I just wanted some proof that it was working here.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            self.context, aggregate, [\u0027an-image\u0027])"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # NOTE(danms): We expect only three image cache attempts because"},{"line_number":63,"context_line":"        # compute5 is marked as forced-down."},{"line_number":64,"context_line":"        self.assertEqual([\u0027an-image\u0027, \u0027an-image\u0027, \u0027an-image\u0027], cached)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        agg_notifications \u003d ["}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_f4843d60","line":63,"updated":"2019-10-09 19:57:54.000000000","message":"and compute2 isn\u0027t in the aggregate.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        # compute5 is marked as forced-down."},{"line_number":64,"context_line":"        self.assertEqual([\u0027an-image\u0027, \u0027an-image\u0027, \u0027an-image\u0027], cached)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        agg_notifications \u003d ["},{"line_number":67,"context_line":"            n[\u0027event_type\u0027] for n in fake_notifier.VERSIONED_NOTIFICATIONS"},{"line_number":68,"context_line":"            if \u0027aggregate\u0027 in n[\u0027event_type\u0027]]"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        self.assertEqual(set([\u0027aggregate.image_cache.start\u0027,"},{"line_number":71,"context_line":"                              \u0027aggregate.image_cache.end\u0027]),"},{"line_number":72,"context_line":"                         set(agg_notifications))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_d45b41f3","line":72,"range":{"start_line":66,"start_character":8,"end_line":72,"end_character":48},"updated":"2019-10-09 19:57:54.000000000","message":"Why not just do:\n\nfake_notifier.wait_for_versioned_notifications(\n    \u0027aggregate.image_cache.start\u0027)\nfake_notifier.wait_for_versioned_notifications(\n    \u0027aggregate.image_cache.end\u0027)\n\n?","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":3621,"context_line":"            mock_image.assert_has_calls([mock.call(self.context,"},{"line_number":3622,"context_line":"                                                   mock.sentinel.image1),"},{"line_number":3623,"context_line":"                                         mock.call(self.context,"},{"line_number":3624,"context_line":"                                               mock.sentinel.image2)])"},{"line_number":3625,"context_line":"            mock_cache.assert_called_once_with("},{"line_number":3626,"context_line":"                self.context, mock.sentinel.aggregate,"},{"line_number":3627,"context_line":"                [mock.sentinel.image1, mock.sentinel.image2])"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_94714979","line":3624,"updated":"2019-10-09 19:57:54.000000000","message":"nit: alignment","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"944a354ae4e3d12347c90e75a53eb9c5f029c335","unresolved":false,"context_lines":[{"line_number":3649,"context_line":"                           \u0027cache_images\u0027)"},{"line_number":3650,"context_line":"        @mock.patch.object(self.conductor.image_api, \u0027get\u0027)"},{"line_number":3651,"context_line":"        def _test(mock_image, mock_cache):"},{"line_number":3652,"context_line":"            mock_image.side_effect \u003d exc.ImageUnacceptable(\u0027foo\u0027)"},{"line_number":3653,"context_line":"            self.assertRaises(exc.ImageUnacceptable,"},{"line_number":3654,"context_line":"                              self.conductor.cache_images,"},{"line_number":3655,"context_line":"                              self.context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_d4290137","line":3652,"range":{"start_line":3652,"start_character":41,"end_line":3652,"end_character":58},"updated":"2019-10-09 19:57:54.000000000","message":"I would use ImageNotFound here to distinguish it from the non-NovaException case being tested above.","commit_id":"16eb42611b0dad38bf3c2b03ea8359a6db280d83"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d813259eaca11b3b7fd50c337a37f2ff1baad09f","unresolved":false,"context_lines":[{"line_number":3619,"context_line":"                                        [mock.sentinel.image1,"},{"line_number":3620,"context_line":"                                         mock.sentinel.image2])"},{"line_number":3621,"context_line":"            mock_image.assert_has_calls([mock.call(self.context,"},{"line_number":3622,"context_line":"                                                mock.sentinel.image1),"},{"line_number":3623,"context_line":"                                         mock.call(self.context,"},{"line_number":3624,"context_line":"                                                   mock.sentinel.image2)])"},{"line_number":3625,"context_line":"            mock_cache.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_3bfcef70","line":3622,"updated":"2019-10-10 20:06:15.000000000","message":"Heh, you fixed the alignment nit from PS4 but introduced this one.","commit_id":"073da99d0cfbf5a50aa8a6f019c17ef477081526"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d813259eaca11b3b7fd50c337a37f2ff1baad09f","unresolved":false,"context_lines":[{"line_number":3646,"context_line":""},{"line_number":3647,"context_line":"        _test()"},{"line_number":3648,"context_line":""},{"line_number":3649,"context_line":"    def test_cache_images_missing(self):"},{"line_number":3650,"context_line":"        @mock.patch.object(self.conductor.conductor_compute_rpcapi,"},{"line_number":3651,"context_line":"                           \u0027cache_images\u0027)"},{"line_number":3652,"context_line":"        @mock.patch.object(self.conductor.image_api, \u0027get\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_bbef7faf","line":3649,"range":{"start_line":3649,"start_character":8,"end_line":3649,"end_character":33},"updated":"2019-10-10 20:06:15.000000000","message":"nit: this and the test above could just be a single test with ddt or a loop of exception classes.","commit_id":"073da99d0cfbf5a50aa8a6f019c17ef477081526"}],"nova/virt/fake.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d813259eaca11b3b7fd50c337a37f2ff1baad09f","unresolved":false,"context_lines":[{"line_number":1015,"context_line":"        return host_status"},{"line_number":1016,"context_line":""},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"class FakeDriverWithCaching(FakeDriver):"},{"line_number":1019,"context_line":"    def __init__(self, *a, **k):"},{"line_number":1020,"context_line":"        super(FakeDriverWithCaching, self).__init__(*a, **k)"},{"line_number":1021,"context_line":"        self.cached_images \u003d set()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_dbecbbbc","line":1018,"range":{"start_line":1018,"start_character":20,"end_line":1018,"end_character":27},"updated":"2019-10-10 20:06:15.000000000","message":"nit: FakeDriverWithImageCaching","commit_id":"073da99d0cfbf5a50aa8a6f019c17ef477081526"}]}