)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1d475003d454a65cc2376d32a29c028798a5341","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Darren Sanders \u003cdarrsand@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-03-11 18:03:10 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t fetch kernel/ramdisk files if you already have them."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"On hard reboot don\u0027t fetch the backing image files from Glance"},{"line_number":10,"context_line":"if nova already has them. This also resolves the problem of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9a80dd14_8019fad2","line":7,"updated":"2015-03-15 06:56:55.000000000","message":"please prefix with libvirt","commit_id":"ca2a2690a57f7127e39093cd5cac0f1455b3c993"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1d475003d454a65cc2376d32a29c028798a5341","unresolved":false,"context_lines":[{"line_number":6966,"context_line":"                          self.test_instance[\u0027project_id\u0027]),"},{"line_number":6967,"context_line":"            ])"},{"line_number":6968,"context_line":""},{"line_number":6969,"context_line":"    @mock.patch.object(os.path, \u0027exists\u0027)"},{"line_number":6970,"context_line":"    def test_create_images_and_backing_images_exist(self, mock_exists):"},{"line_number":6971,"context_line":"        mock_exists.return_value \u003d True"},{"line_number":6972,"context_line":"        conn \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_a07b5ef3","line":6969,"updated":"2015-03-15 06:56:55.000000000","message":"@mock.patch.object(os.path, \u0027exists\u0027, return_value\u003dTrue)\nthen you do not need line 6971","commit_id":"ca2a2690a57f7127e39093cd5cac0f1455b3c993"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f1d475003d454a65cc2376d32a29c028798a5341","unresolved":false,"context_lines":[{"line_number":6984,"context_line":"                                   \u0027project_id\u0027: \u0027fake-project\u0027})"},{"line_number":6985,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":6986,"context_line":""},{"line_number":6987,"context_line":"        with mock.patch.object(libvirt_driver.libvirt_utils, \u0027fetch_image\u0027,"},{"line_number":6988,"context_line":"                               side_effect\u003dexception.ImageNotFound("},{"line_number":6989,"context_line":"                                   image_id\u003d\"fake_id\")):"},{"line_number":6990,"context_line":"            result \u003d conn._create_images_and_backing(self.context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_e081d6c3","line":6987,"updated":"2015-03-15 06:56:55.000000000","message":"why do we need this mock defined here?\nCan it also be a decorator?","commit_id":"ca2a2690a57f7127e39093cd5cac0f1455b3c993"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4b8cc2518fa9f521ec44b1e505f6dc1dea3d2e54","unresolved":false,"context_lines":[{"line_number":6989,"context_line":"                                                 instance,"},{"line_number":6990,"context_line":"                                                 \u0027/fake/instance/dir\u0027,"},{"line_number":6991,"context_line":"                                                 disk_info_json)"},{"line_number":6992,"context_line":"        self.assertIsNone(result)"},{"line_number":6993,"context_line":""},{"line_number":6994,"context_line":"    def test_create_images_and_backing_ephemeral_gets_created(self):"},{"line_number":6995,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aa53dc9_69d92f83","line":6992,"updated":"2015-04-08 01:26:44.000000000","message":"I think instead of asserting result is None (for a function that has no return value), it would be better to self.assertFalse(mock_fetch_image.called) and get rid of the \"result\" variable. It would also make it so you don\u0027t need to specify a side_effect for the mock_fetch_image.\n\nAnother thing is this doesn\u0027t exercise the code path if kernel_path doesn\u0027t exist but ramdisk_path does. For that, you can have another similar test where you set side_effect\u003d[False, True] for mock_exists.","commit_id":"b403525c56b0ab2420c4505ccf6b1cf4352d05d9"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"44585c7d246fcf2572a5212372a5420d040f9b46","unresolved":false,"context_lines":[{"line_number":6989,"context_line":"                                                 instance,"},{"line_number":6990,"context_line":"                                                 \u0027/fake/instance/dir\u0027,"},{"line_number":6991,"context_line":"                                                 disk_info_json)"},{"line_number":6992,"context_line":"        self.assertIsNone(result)"},{"line_number":6993,"context_line":""},{"line_number":6994,"context_line":"    def test_create_images_and_backing_ephemeral_gets_created(self):"},{"line_number":6995,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_c241b0aa","line":6992,"in_reply_to":"7a016987_37bba2e7","updated":"2015-06-17 18:41:54.000000000","message":"Done","commit_id":"b403525c56b0ab2420c4505ccf6b1cf4352d05d9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"4a25eac39f4c46ce00b4f350117a232b87853448","unresolved":false,"context_lines":[{"line_number":6989,"context_line":"                                                 instance,"},{"line_number":6990,"context_line":"                                                 \u0027/fake/instance/dir\u0027,"},{"line_number":6991,"context_line":"                                                 disk_info_json)"},{"line_number":6992,"context_line":"        self.assertIsNone(result)"},{"line_number":6993,"context_line":""},{"line_number":6994,"context_line":"    def test_create_images_and_backing_ephemeral_gets_created(self):"},{"line_number":6995,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_37bba2e7","line":6992,"in_reply_to":"9aa53dc9_69d92f83","updated":"2015-05-15 13:29:05.000000000","message":"Agreed, asserting  on mock_fech_image.called makes it clearer exactly what this test case is checking for.","commit_id":"b403525c56b0ab2420c4505ccf6b1cf4352d05d9"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"44585c7d246fcf2572a5212372a5420d040f9b46","unresolved":false,"context_lines":[{"line_number":6989,"context_line":"                                                 instance,"},{"line_number":6990,"context_line":"                                                 \u0027/fake/instance/dir\u0027,"},{"line_number":6991,"context_line":"                                                 disk_info_json)"},{"line_number":6992,"context_line":"        self.assertIsNone(result)"},{"line_number":6993,"context_line":""},{"line_number":6994,"context_line":"    def test_create_images_and_backing_ephemeral_gets_created(self):"},{"line_number":6995,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_22389c3c","line":6992,"in_reply_to":"9aa53dc9_69d92f83","updated":"2015-06-17 18:41:54.000000000","message":"Done","commit_id":"b403525c56b0ab2420c4505ccf6b1cf4352d05d9"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"45dc9a41e2fbc62dcb3e66814969da0bebb15e7d","unresolved":false,"context_lines":[{"line_number":5657,"context_line":"            LOG.debug(\"Live migration monitoring is all done\","},{"line_number":5658,"context_line":"                      instance\u003dinstance)"},{"line_number":5659,"context_line":""},{"line_number":5660,"context_line":"    def _try_fetch_image(self, context, path, image_id, instance,"},{"line_number":5661,"context_line":"                         fallback_from_host\u003dNone):"},{"line_number":5662,"context_line":"        try:"},{"line_number":5663,"context_line":"            libvirt_utils.fetch_image(context, path,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_a4709bdd","line":5660,"updated":"2015-03-16 21:12:41.000000000","message":"instead of adding checks below, is it possible to add\n\nif not os.path.exists(path):\n       return \n\nhere?  to avoid dupliate check and comments?","commit_id":"b403525c56b0ab2420c4505ccf6b1cf4352d05d9"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"74f1e133e2e07cc77e6f8a2023952965169cf920","unresolved":false,"context_lines":[{"line_number":5657,"context_line":"            LOG.debug(\"Live migration monitoring is all done\","},{"line_number":5658,"context_line":"                      instance\u003dinstance)"},{"line_number":5659,"context_line":""},{"line_number":5660,"context_line":"    def _try_fetch_image(self, context, path, image_id, instance,"},{"line_number":5661,"context_line":"                         fallback_from_host\u003dNone):"},{"line_number":5662,"context_line":"        try:"},{"line_number":5663,"context_line":"            libvirt_utils.fetch_image(context, path,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_865d4818","line":5660,"in_reply_to":"9a80dd14_5f632641","updated":"2015-03-17 09:04:35.000000000","message":"#1 ok ,I missed the logic \n\n#2 the function is only used by\n_fetch_instance_kernel_ramdisk\n     what ever the result after this function is \n     to guarantee the image is there \n\n     Maybe it\u0027s a both acceptable case but I only want to be as simple as we can\n\nI took back my -1, thanks for the explaintion","commit_id":"b403525c56b0ab2420c4505ccf6b1cf4352d05d9"},{"author":{"_account_id":14847,"name":"Darren Sanders","email":"darrsand@cisco.com","username":"darren"},"change_message_id":"f2f257ab5c9a3b9cd3dcf85fcca052aa5ce1c4d5","unresolved":false,"context_lines":[{"line_number":5657,"context_line":"            LOG.debug(\"Live migration monitoring is all done\","},{"line_number":5658,"context_line":"                      instance\u003dinstance)"},{"line_number":5659,"context_line":""},{"line_number":5660,"context_line":"    def _try_fetch_image(self, context, path, image_id, instance,"},{"line_number":5661,"context_line":"                         fallback_from_host\u003dNone):"},{"line_number":5662,"context_line":"        try:"},{"line_number":5663,"context_line":"            libvirt_utils.fetch_image(context, path,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_5f632641","line":5660,"in_reply_to":"9a80dd14_a4709bdd","updated":"2015-03-16 21:37:24.000000000","message":"No, this would not be the correct place to add the check.\n\nFirst of all, your check is incorrect. It would need to be if os.path.exists: return. Not, \"if not exists, return\".\n\nSecondly, by putting the check (minus the \u0027not\u0027) in this method, you would be fundamentally changing the behavior of a core function for all callers, not just for the _fetch_instance_kernel_ramdisk caller. So while this check here  would work for the _fetch_instance_kernel_ramdisk, it would represent a non-obvious, change (and side-effect) to a simple function that is meant to do just one thing.","commit_id":"b403525c56b0ab2420c4505ccf6b1cf4352d05d9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e8fca29a2989deee7b727c96f7856cbf6af7b7dd","unresolved":false,"context_lines":[{"line_number":5787,"context_line":"            # NOTE(dsanders): only fetch image if it\u0027s not available at"},{"line_number":5788,"context_line":"            # kernel_path. This also avoids ImageNotFound exception if"},{"line_number":5789,"context_line":"            # the image has been deleted from glance"},{"line_number":5790,"context_line":"            if not os.path.exists(kernel_path):"},{"line_number":5791,"context_line":"                self._try_fetch_image(context,"},{"line_number":5792,"context_line":"                                      kernel_path,"},{"line_number":5793,"context_line":"                                      instance.kernel_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_1eb3570b","line":5790,"updated":"2015-06-18 11:11:01.000000000","message":"should we not do:\nif kernel_path and not os.path.exists(kernel_path):\n\nthis will return an error if the kernel path is None\n\nTypeError: coercing to Unicode: need string or buffer, NoneType found","commit_id":"5fdbc850e39d1d346095e9d168fb1ca22735d53a"},{"author":{"_account_id":14847,"name":"Darren Sanders","email":"darrsand@cisco.com","username":"darren"},"change_message_id":"c55390d73d7218ea7fb1dfac6390e2f4d7100632","unresolved":false,"context_lines":[{"line_number":5787,"context_line":"            # NOTE(dsanders): only fetch image if it\u0027s not available at"},{"line_number":5788,"context_line":"            # kernel_path. This also avoids ImageNotFound exception if"},{"line_number":5789,"context_line":"            # the image has been deleted from glance"},{"line_number":5790,"context_line":"            if not os.path.exists(kernel_path):"},{"line_number":5791,"context_line":"                self._try_fetch_image(context,"},{"line_number":5792,"context_line":"                                      kernel_path,"},{"line_number":5793,"context_line":"                                      instance.kernel_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_e9b4ad82","line":5790,"in_reply_to":"fa32b979_1eb3570b","updated":"2015-06-18 16:00:20.000000000","message":"I don\u0027t see any way for kernel_path (or ramdisk_path) to be None at this point in the code. My understanding is os.path.join will either return a string ending in \u0027kernel\u0027 or will throw an exception, but under no circumstance that I\u0027m aware of can it return None as it\u0027s being used here. Can you explain a scenario that could result in this? Thanks.","commit_id":"5fdbc850e39d1d346095e9d168fb1ca22735d53a"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"0c99a0a81d4a9c27da67666e6d3bd0e2f6930e53","unresolved":false,"context_lines":[{"line_number":5787,"context_line":"            # NOTE(dsanders): only fetch image if it\u0027s not available at"},{"line_number":5788,"context_line":"            # kernel_path. This also avoids ImageNotFound exception if"},{"line_number":5789,"context_line":"            # the image has been deleted from glance"},{"line_number":5790,"context_line":"            if not os.path.exists(kernel_path):"},{"line_number":5791,"context_line":"                self._try_fetch_image(context,"},{"line_number":5792,"context_line":"                                      kernel_path,"},{"line_number":5793,"context_line":"                                      instance.kernel_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_c0573656","line":5790,"in_reply_to":"fa32b979_1eb3570b","updated":"2015-06-18 17:34:15.000000000","message":"Whaaa?  kernel_path is bound to an lvalue on the previous (non-comment) line of code.  And instance_dir is bound to an lvalue right before that.\n\nYou\u0027re asking him to armor against a situation that is not at all reasonably possible.","commit_id":"5fdbc850e39d1d346095e9d168fb1ca22735d53a"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e8fca29a2989deee7b727c96f7856cbf6af7b7dd","unresolved":false,"context_lines":[{"line_number":5793,"context_line":"                                      instance.kernel_id,"},{"line_number":5794,"context_line":"                                      instance, fallback_from_host)"},{"line_number":5795,"context_line":"            if instance.ramdisk_id:"},{"line_number":5796,"context_line":"                ramdisk_path \u003d os.path.join(instance_dir, \u0027ramdisk\u0027)"},{"line_number":5797,"context_line":"                # NOTE(dsanders): only fetch image if it\u0027s not available at"},{"line_number":5798,"context_line":"                # ramdisk_path. This also avoids ImageNotFound exception if"},{"line_number":5799,"context_line":"                # the image has been deleted from glance"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_9e9e8761","line":5796,"updated":"2015-06-18 11:11:01.000000000","message":"same point here","commit_id":"5fdbc850e39d1d346095e9d168fb1ca22735d53a"}]}