)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f9decd0b88638fec6ae11a224763febf4ba011b","unresolved":false,"context_lines":[{"line_number":7,"context_line":"xenapi: Adding BitTorrent download handler"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Part of series of patches which extends uploadhandler driver-pattern to also"},{"line_number":10,"context_line":"handle downloads."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"DocImpact"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAQn%2F%2FN14%3D","line":10,"updated":"2013-08-05 11:02:31.000000000","message":"I would be tempted to mention the new config param in here.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"735a5bcb29fad4885265af11278128ce552339a3","unresolved":false,"context_lines":[{"line_number":7,"context_line":"xenapi: Adding BitTorrent download handler"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Part of series of patches which extends uploadhandler driver-pattern to also"},{"line_number":10,"context_line":"handle downloads."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"DocImpact"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAQn%2F%2FLgk%3D","line":10,"in_reply_to":"AAAAQn%2F%2FN14%3D","updated":"2013-08-05 21:20:44.000000000","message":"Good point.\n\nThinking about breaking out the configuration changes to another patch...","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"}],"nova/virt/xenapi/image/bittorrent.py":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"0e46a604f249b278286431fe898a995032464eb4","unresolved":false,"context_lines":[{"line_number":42,"context_line":"_TORRENT_URL_FN \u003d None  # driver function to determine torrent URL to use"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def _lookup_torrent_url_fn():"},{"line_number":46,"context_line":"    \"\"\"Load a \"fetcher\" func to get the right torrent URL via entrypoints.\"\"\""},{"line_number":47,"context_line":"    global _TORRENT_URL_FN"},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fjw4%3D","line":45,"updated":"2013-07-16 15:44:12.000000000","message":"might as well move this into the \"Store\" class below","commit_id":"706beb2c510e2f4c1d7fc3f2c1860cf85db55771"}],"nova/virt/xenapi/vm_utils.py":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"0e46a604f249b278286431fe898a995032464eb4","unresolved":false,"context_lines":[{"line_number":1138,"context_line":"    LOG.debug(_(\"Asking xapi to fetch vhd image %s\"), image_id,"},{"line_number":1139,"context_line":"              instance\u003dinstance)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"    if _image_uses_bittorrent(context, instance):"},{"line_number":1142,"context_line":"        store \u003d \u0027nova.virt.xenapi.image.bittorrent.BittorrentStore\u0027"},{"line_number":1143,"context_line":"    else:"},{"line_number":1144,"context_line":"        store \u003d \u0027nova.virt.xenapi.image.glance.GlanceStore\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fjxk%3D","line":1141,"updated":"2013-07-16 15:44:12.000000000","message":"Perhaps a general pattern here:\n\ndownload_handler \u003d _choose_handler(context, instance)\ntry:\n  download_handler.download_image(...)\nexcept DownloadHandlerUnavailble:\n  default_handler.download_image(...)","commit_id":"706beb2c510e2f4c1d7fc3f2c1860cf85db55771"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f9decd0b88638fec6ae11a224763febf4ba011b","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    cfg.IntOpt(\u0027xenapi_num_vbd_unplug_retries\u0027,"},{"line_number":94,"context_line":"               default\u003d10,"},{"line_number":95,"context_line":"               help\u003d\u0027Maximum number of retries to unplug VBD\u0027),"},{"line_number":96,"context_line":"    cfg.StrOpt(\u0027xenapi_torrent_images\u0027,"},{"line_number":97,"context_line":"               default\u003d\u0027none\u0027,"},{"line_number":98,"context_line":"               help\u003d\u0027Whether or not to download images via Bit Torrent \u0027"},{"line_number":99,"context_line":"                    \u0027(all|some|none).\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FN0k%3D","line":96,"updated":"2013-08-05 11:02:31.000000000","message":"Not sure all these settings make sense.\n\nMaybe have a glance hander and bittorrent hander setting, or just hard code them.\n\nThen use the xenapi_torrent_images to select between them.\n\nA setting of \u0027none\u0027 and \u0027bittorret\u0027, for example, just makes no logical sense, but would work, and make everything download with bittorent.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"ae99fc12327d0a25fdcd436ca2784f74f92504e5","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    cfg.IntOpt(\u0027xenapi_num_vbd_unplug_retries\u0027,"},{"line_number":94,"context_line":"               default\u003d10,"},{"line_number":95,"context_line":"               help\u003d\u0027Maximum number of retries to unplug VBD\u0027),"},{"line_number":96,"context_line":"    cfg.StrOpt(\u0027xenapi_torrent_images\u0027,"},{"line_number":97,"context_line":"               default\u003d\u0027none\u0027,"},{"line_number":98,"context_line":"               help\u003d\u0027Whether or not to download images via Bit Torrent \u0027"},{"line_number":99,"context_line":"                    \u0027(all|some|none).\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FLeE%3D","line":96,"in_reply_to":"AAAAQn%2F%2FLgw%3D","updated":"2013-08-05 21:29:42.000000000","message":"I think you want a priority order of handlers and you go through each one checking to see if it should be used.\n\nYou just need more than a boolean flag to decide which handler to use.  Once you get rid of that, the awkward xenapi_torrent_images flag can be hidden behind a method in the handle.  Something like use_handler() that returns True or False if the handler should be used or the next handler in the list should be queried.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"735a5bcb29fad4885265af11278128ce552339a3","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    cfg.IntOpt(\u0027xenapi_num_vbd_unplug_retries\u0027,"},{"line_number":94,"context_line":"               default\u003d10,"},{"line_number":95,"context_line":"               help\u003d\u0027Maximum number of retries to unplug VBD\u0027),"},{"line_number":96,"context_line":"    cfg.StrOpt(\u0027xenapi_torrent_images\u0027,"},{"line_number":97,"context_line":"               default\u003d\u0027none\u0027,"},{"line_number":98,"context_line":"               help\u003d\u0027Whether or not to download images via Bit Torrent \u0027"},{"line_number":99,"context_line":"                    \u0027(all|some|none).\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FLgw%3D","line":96,"in_reply_to":"AAAAQn%2F%2FN0k%3D","updated":"2013-08-05 21:20:44.000000000","message":"Yeah agree the configuration isn\u0027t ideal at the moment. Maybe the right answer is to hard-code everything in this patch, and have a separate patch attempt to \u0027do-configuration-of-image-handlers-right\u0027.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f9decd0b88638fec6ae11a224763febf4ba011b","unresolved":false,"context_lines":[{"line_number":1112,"context_line":""},{"line_number":1113,"context_line":""},{"line_number":1114,"context_line":"def _default_download_handler():"},{"line_number":1115,"context_line":"    return importutils.import_object(CONF.xenapi_image_download_handler)"},{"line_number":1116,"context_line":""},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"def _choose_download_handler(context, instance):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FN08%3D","line":1115,"updated":"2013-08-05 11:02:31.000000000","message":"Can we move these handlers into VM ops so we only need to do the import once? Or maybe cache them in a module variable?","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"07072c18aca56ed4488fed88f5cf5114713956d9","unresolved":false,"context_lines":[{"line_number":1112,"context_line":""},{"line_number":1113,"context_line":""},{"line_number":1114,"context_line":"def _default_download_handler():"},{"line_number":1115,"context_line":"    return importutils.import_object(CONF.xenapi_image_download_handler)"},{"line_number":1116,"context_line":""},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"def _choose_download_handler(context, instance):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FLEg%3D","line":1115,"in_reply_to":"AAAAQn%2F%2FLhY%3D","updated":"2013-08-06 00:04:52.000000000","message":"Looking into this any more, I don\u0027t think we need to cache separately since `__import__` checks sys.modules to see if the module is imported already before trying to re-import, so it\u0027s already cached.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"735a5bcb29fad4885265af11278128ce552339a3","unresolved":false,"context_lines":[{"line_number":1112,"context_line":""},{"line_number":1113,"context_line":""},{"line_number":1114,"context_line":"def _default_download_handler():"},{"line_number":1115,"context_line":"    return importutils.import_object(CONF.xenapi_image_download_handler)"},{"line_number":1116,"context_line":""},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"def _choose_download_handler(context, instance):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FLhY%3D","line":1115,"in_reply_to":"AAAAQn%2F%2FN08%3D","updated":"2013-08-05 21:20:44.000000000","message":"Yeah I\u0027ll look into that. (Have a feeling it might be harder than it appears b/c of weird cyclical dependency issues. But I\u0027ll see...)","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f9decd0b88638fec6ae11a224763febf4ba011b","unresolved":false,"context_lines":[{"line_number":1118,"context_line":"def _choose_download_handler(context, instance):"},{"line_number":1119,"context_line":"    if _image_uses_bittorrent(context, instance):"},{"line_number":1120,"context_line":"        return importutils.import_object("},{"line_number":1121,"context_line":"                \u0027nova.virt.xenapi.image.bittorrent.BittorrentStore\u0027)"},{"line_number":1122,"context_line":"    else:"},{"line_number":1123,"context_line":"        return _default_download_handler()"},{"line_number":1124,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FN1M%3D","line":1121,"updated":"2013-08-05 11:02:31.000000000","message":"If we hard code this, do want want the default hander setting to just be something like \"glance\" vs \"bittorrent\u0027?","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"735a5bcb29fad4885265af11278128ce552339a3","unresolved":false,"context_lines":[{"line_number":1118,"context_line":"def _choose_download_handler(context, instance):"},{"line_number":1119,"context_line":"    if _image_uses_bittorrent(context, instance):"},{"line_number":1120,"context_line":"        return importutils.import_object("},{"line_number":1121,"context_line":"                \u0027nova.virt.xenapi.image.bittorrent.BittorrentStore\u0027)"},{"line_number":1122,"context_line":"    else:"},{"line_number":1123,"context_line":"        return _default_download_handler()"},{"line_number":1124,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FLhk%3D","line":1121,"in_reply_to":"AAAAQn%2F%2FN1M%3D","updated":"2013-08-05 21:20:44.000000000","message":"So right now the code attempts to find a BitTorrent URL for the image and falls back to downloading from Glance.\n\nSo the idea here was to add a \u0027default-handler\u0027 to handle that fall back case. In the new code, we attempt to download from BitTorrent (or any other image-handler), and then if an error occurs we fall back to  the default image-handler.\n\nMaybe along these lines, we could have a default_download_handler and a preferred_download_handler which would keep us from having to hard code that.\n\nAll that said, overall, I\u0027m more inclined to merge this with a bit of inconsistency in the settings (since this is principally a refactoring not a change in behavior) and then have a follow along patch figure out how to better handle the configuration of the image-handlers.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"14f1c497e3b263a69e425700ad05f986270f9caa","unresolved":false,"context_lines":[{"line_number":1109,"context_line":""},{"line_number":1110,"context_line":""},{"line_number":1111,"context_line":"def _default_download_handler():"},{"line_number":1112,"context_line":"    # TODO(sirp):  This should be configurable like upload_handler"},{"line_number":1113,"context_line":"    return importutils.import_object("},{"line_number":1114,"context_line":"            \u0027nova.virt.xenapi.image.glance.GlanceStore\u0027)"},{"line_number":1115,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAQn%2F%2FEIU%3D","line":1112,"updated":"2013-08-07 17:48:59.000000000","message":"I think hard coding for now, and figuring out some better we to configure this is a good call!","commit_id":"f5c56e5141b39df2011b01fd8cfead5f8613da12"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f9decd0b88638fec6ae11a224763febf4ba011b","unresolved":false,"context_lines":[{"line_number":67,"context_line":"               help\u003d\u0027The XenAPI VIF driver using XenServer Network APIs.\u0027),"},{"line_number":68,"context_line":"    cfg.StrOpt(\u0027xenapi_image_upload_handler\u0027,"},{"line_number":69,"context_line":"                default\u003d\u0027nova.virt.xenapi.image.glance.GlanceStore\u0027,"},{"line_number":70,"context_line":"                help\u003d\u0027Dom0 plugin driver used to handle image uploads.\u0027),"},{"line_number":71,"context_line":"    ]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FN1Y%3D","line":70,"updated":"2013-08-05 11:02:31.000000000","message":"Would be nice to have the download hander here too, I think.\n\nThey are a nice logical pair.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"735a5bcb29fad4885265af11278128ce552339a3","unresolved":false,"context_lines":[{"line_number":67,"context_line":"               help\u003d\u0027The XenAPI VIF driver using XenServer Network APIs.\u0027),"},{"line_number":68,"context_line":"    cfg.StrOpt(\u0027xenapi_image_upload_handler\u0027,"},{"line_number":69,"context_line":"                default\u003d\u0027nova.virt.xenapi.image.glance.GlanceStore\u0027,"},{"line_number":70,"context_line":"                help\u003d\u0027Dom0 plugin driver used to handle image uploads.\u0027),"},{"line_number":71,"context_line":"    ]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FLjo%3D","line":70,"in_reply_to":"AAAAQn%2F%2FN1Y%3D","updated":"2013-08-05 21:20:44.000000000","message":"Agreed. I originally tried to do that, but ran into circular-dependency issues with how the configs are imported. Can\u0027t remember the exact details, but the way the code is broken down now, try to do an `CFG.import_opt` in `vm_utils` of `vmops` causes massive breakage.\n\nI\u0027ll spend some more time looking into how to get this done.","commit_id":"c38fc4d81708a7e8e17c4924356e1ef679d67ac0"}]}