)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"70b77c565ce9eb92fd788b860cbc35840f70fb35","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Supports import of OVA package to Glance by import task."},{"line_number":10,"context_line":"Automatically extracts single disk image from OVA tarball for saving"},{"line_number":11,"context_line":"and parses included OVF file for interested metadata. Metadata is"},{"line_number":12,"context_line":"included as uploaded image\u0027s properties."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Uses existing Glance import task. Specify \u0027ova\u0027 as \u0027container_format\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"da20952f_aca61ba9","line":11,"updated":"2015-09-10 22:35:41.000000000","message":"IMO, the whole metadata extraction could go in a dependent patch. It will make it easier to review \u0026 add unit tests.\n\nThis patch can just focus on untar-ing the root disk from OVA.","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"}],"etc/ovf-metadata.json.sample":[{"author":{"_account_id":17561,"name":"Jakub Jasek","email":"jakub.jasek@tieto.com","username":"jjasek"},"change_message_id":"43cfcfc4631735b7e2108757e3051aaa15aa547c","unresolved":false,"context_lines":[{"line_number":2,"context_line":"    \"ovf_metadata_properties\": ["},{"line_number":3,"context_line":"        \"ProcessorArchitecture\","},{"line_number":4,"context_line":"        \"InstructionSet\","},{"line_number":5,"context_line":"        \"InstructionSetExtensionName\","},{"line_number":6,"context_line":"    ]"},{"line_number":7,"context_line":"}"},{"line_number":8,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":12,"id":"ba15a1d1_e0daa14a","line":5,"updated":"2015-09-17 12:17:38.000000000","message":"This is not valid json file. Pls remove comma at the and of this line.","commit_id":"eb2e2be427f2d3f399fb20c52497b79116c19305"}],"glance/async/flows/base_import.py":[{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"5901cc1e9318c48d0ec0b3c5dafa40efae1fef58","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    extensions \u003d named.NamedExtensionManager(\u0027glance.flows.import\u0027,"},{"line_number":389,"context_line":"                                             names\u003d[\u0027convert\u0027,"},{"line_number":390,"context_line":"                                                    \u0027introspect\u0027,"},{"line_number":391,"context_line":"                                                    \u0027ovf_process\u0027],"},{"line_number":392,"context_line":"                                             name_order\u003dTrue,"},{"line_number":393,"context_line":"                                             invoke_on_load\u003dTrue,"},{"line_number":394,"context_line":"                                             invoke_kwds\u003dkwargs)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_d0f0d968","line":391,"updated":"2015-09-10 14:06:34.000000000","message":"You also need to add `ovf_process` in the setup.cfg file so that the entry point will be installed.\n\nhttps://github.com/openstack/glance/blob/master/setup.cfg#L68","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"70b77c565ce9eb92fd788b860cbc35840f70fb35","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    extensions \u003d named.NamedExtensionManager(\u0027glance.flows.import\u0027,"},{"line_number":389,"context_line":"                                             names\u003d[\u0027convert\u0027,"},{"line_number":390,"context_line":"                                                    \u0027introspect\u0027,"},{"line_number":391,"context_line":"                                                    \u0027ovf_process\u0027],"},{"line_number":392,"context_line":"                                             name_order\u003dTrue,"},{"line_number":393,"context_line":"                                             invoke_on_load\u003dTrue,"},{"line_number":394,"context_line":"                                             invoke_kwds\u003dkwargs)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_d77be5bc","line":391,"updated":"2015-09-10 22:35:41.000000000","message":"You need to move ovf_process above convert so that the extracted disk is passed to the convert task.","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"f28fcadb11b39db4ceae61963e5a9b56d71e84e1","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    extensions \u003d named.NamedExtensionManager(\u0027glance.flows.import\u0027,"},{"line_number":389,"context_line":"                                             names\u003d[\u0027convert\u0027,"},{"line_number":390,"context_line":"                                                    \u0027introspect\u0027,"},{"line_number":391,"context_line":"                                                    \u0027ovf_process\u0027],"},{"line_number":392,"context_line":"                                             name_order\u003dTrue,"},{"line_number":393,"context_line":"                                             invoke_on_load\u003dTrue,"},{"line_number":394,"context_line":"                                             invoke_kwds\u003dkwargs)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_006836b9","line":391,"in_reply_to":"da20952f_d0f0d968","updated":"2015-09-10 17:26:18.000000000","message":"Got it, Thanks. Where would we determine the ordering for the tasks? Sabari mentioned that the \u0027ovf_process\u0027 should go after \u0027ImportToFS\u0027 and before \u0027convert\u0027, but I\u0027m not sure where to specify this.","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"}],"glance/async/flows/ovf_process.py":[{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"70b77c565ce9eb92fd788b860cbc35840f70fb35","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"class _OVF_Process(utils.OptionalTask):"},{"line_number":43,"context_line":"    \"\"\"Taskflow to extract and parse OVA and OVF\"\"\""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def __init__(self, task_id, task_type, image_repo):"},{"line_number":46,"context_line":"        self.task_id \u003d task_id"},{"line_number":47,"context_line":"        self.task_type \u003d task_type"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_b7f19197","line":44,"updated":"2015-09-10 22:35:41.000000000","message":"default_provides \u003d \u0027file_path\u0027","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"70b77c565ce9eb92fd788b860cbc35840f70fb35","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        super(_OVF_Process, self).__init__("},{"line_number":50,"context_line":"            name\u003d\u0027%s-OVF_Process-%s\u0027 % (task_type, task_id))"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def execute(self, image_id, file_path):"},{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        :param image_id: Glance image ID"},{"line_number":55,"context_line":"        :param file_path: Path to the file"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_f725c909","line":52,"updated":"2015-09-10 22:35:41.000000000","message":"This method should return a file_path of the extracted disk","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"743ba53dca0bda2541f5941eff028ab4dc67d1f8","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        super(_OVF_Process, self).__init__("},{"line_number":50,"context_line":"            name\u003d\u0027%s-OVF_Process-%s\u0027 % (task_type, task_id))"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def execute(self, image_id, file_path):"},{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        :param image_id: Glance image ID"},{"line_number":55,"context_line":"        :param file_path: Path to the file"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_dd5d12ed","line":52,"in_reply_to":"da20952f_86120bb3","updated":"2015-09-11 21:24:58.000000000","message":"Kent, adding to the backend will be taken care by ImportToStore Task. \n\nYou just need to extract the disk to the work_dir (take a look at the convert task) and then return a file_path to the same. All future tasks (inc. ImportToStore) in the flow will consume this file_path for their respective operations.","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"6ddcf9a50bff76ed4e036e0f6be815eb51640fb5","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        super(_OVF_Process, self).__init__("},{"line_number":50,"context_line":"            name\u003d\u0027%s-OVF_Process-%s\u0027 % (task_type, task_id))"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def execute(self, image_id, file_path):"},{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        :param image_id: Glance image ID"},{"line_number":55,"context_line":"        :param file_path: Path to the file"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_86120bb3","line":52,"in_reply_to":"da20952f_f725c909","updated":"2015-09-11 06:04:27.000000000","message":"A little confused here. So after extracting the disk, would we call glance_store.add_to_backend(...) for the disk? (would also need all the initializations). Have a question. Since this is what _ImportToFS does, is it better to call _ImportToFS here on the extracted disk? (although this would muck up the taskflow chain I think).","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"70b77c565ce9eb92fd788b860cbc35840f70fb35","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        :returns: a tuple of extracted disk file object and dictionary of"},{"line_number":97,"context_line":"                  properties parsed from the OVF file"},{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        with tarfile.open(fileobj\u003dova) as tar_file:"},{"line_number":100,"context_line":"            disk_name, properties \u003d None, None"},{"line_number":101,"context_line":"            filenames \u003d tar_file.getnames()"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_6c300357","line":99,"updated":"2015-09-10 22:35:41.000000000","message":"It would be better to process the tarfile sequentially for all the reasons I mentioned in a earlier review.","commit_id":"63679ba941b2ab59525a52d374a8ede427e63a55"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import tarfile"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"try:"},{"line_number":19,"context_line":"    import xml.etree.cElementTree as ET"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_1420d79a","line":16,"updated":"2015-09-14 10:41:01.000000000","message":"Please keep the imports in 3 main blocks like specified in hacking documentation: http://docs.openstack.org/developer/hacking/#real-world-import-order-examples","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":20,"context_line":"except ImportError:"},{"line_number":21,"context_line":"    import xml.etree.ElementTree as ET"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import logging"},{"line_number":24,"context_line":"import os"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from oslo_config import cfg"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_f43e83b7","line":23,"updated":"2015-09-14 10:41:01.000000000","message":"Is there reason why we use this rather than oslo_log?","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            else:"},{"line_number":78,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"},{"line_number":79,"context_line":"        else:"},{"line_number":80,"context_line":"            LOG.error(_LE(\"Expected \u0027ova\u0027 image container format for \""},{"line_number":81,"context_line":"                          \u0027OVF_Process task\u0027))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        return file_path"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_b417abec","line":80,"updated":"2015-09-14 10:41:01.000000000","message":"This is definitely not worth of ERROR level logging. https://wiki.openstack.org/wiki/LoggingStandards#Log_level_definitions","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            if ovf_filename:"},{"line_number":115,"context_line":"                ovf \u003d tar_file.extractfile(ovf_filename)"},{"line_number":116,"context_line":"                properties \u003d self._parse_OVF(ovf)"},{"line_number":117,"context_line":"                ovf.close()"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"            for filename in filenames:"},{"line_number":120,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_b446cbbd","line":117,"updated":"2015-09-14 10:41:01.000000000","message":"We seriously should not process any further if the ovf file was not found as the OVA standard specifies that there is always exactly one.","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                ovf.close()"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"            for filename in filenames:"},{"line_number":120,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"},{"line_number":121,"context_line":"                    disk_name \u003d filename"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_545e4fea","line":120,"updated":"2015-09-14 10:41:01.000000000","message":"And we just should not treat file extension as anything else than human readable nowadays. As there is way to determine the disk image from the OVF data, it\u0027s really important to use that.","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":4992,"name":"malini-k-bhandaru","email":"malini.k.bhandaru@intel.com","username":"malini-k-bhandaru"},"change_message_id":"2195bd9458ca3e01831d3bbe8abd93ae4a6b33b0","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                ovf.close()"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"            for filename in filenames:"},{"line_number":120,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"},{"line_number":121,"context_line":"                    disk_name \u003d filename"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_b91d6911","line":120,"updated":"2015-09-14 08:36:14.000000000","message":"regardless on stream processing or extracted tar processing, based on conversation with Sabari, need to be able to handle OVA with multiple disk images, and here extract the first disk. Essentially need to figure out which is the first disk, which can be done only by parsing the OVF file. And then seeking its contents.","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":122,"context_line":""},{"line_number":123,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"},{"line_number":124,"context_line":"            if not disk:"},{"line_number":125,"context_line":"                LOG.error(_LE(\u0027Could not find valid disk image in OVA \u0027"},{"line_number":126,"context_line":"                              \u0027archive file\u0027))"},{"line_number":127,"context_line":"            return (disk, properties)"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_342b9b32","line":125,"updated":"2015-09-14 10:41:01.000000000","message":"This is most definitely not worth of ERROR level logging. https://wiki.openstack.org/wiki/LoggingStandards#Log_level_definitions","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        OVF properties config file specifies which metadata of interest to"},{"line_number":159,"context_line":"        extract. Reads in a JSON file named \u0027ovf-metadata.json\u0027 if available."},{"line_number":160,"context_line":"        Example file:"},{"line_number":161,"context_line":"            {"},{"line_number":162,"context_line":"                \"ovf_metadata_properties\": ["},{"line_number":163,"context_line":"                    \"ProcessorArchitecture\","}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_b4c56b5c","line":160,"updated":"2015-09-14 10:41:01.000000000","message":"How about pointing to etc/ovf-metadata.json.sample rather than copying the content here and having a need to keep these two in sync?","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":176,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":177,"context_line":"                if not self.interested_properties:"},{"line_number":178,"context_line":"                    LOG.warn(_LW(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":179,"context_line":"                                 \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":180,"context_line":"                                 \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":181,"context_line":"                                 \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_d4a33f9d","line":178,"updated":"2015-09-14 10:41:01.000000000","message":"This in the other hand is worth of ERROR if the flow is enabled but not configured.","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c02222a9a456d0f2148460c63f3266763860fcb7","unresolved":false,"context_lines":[{"line_number":180,"context_line":"                                 \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":181,"context_line":"                                 \u0027interested properties\u0027))"},{"line_number":182,"context_line":"        else:"},{"line_number":183,"context_line":"            LOG.warn(_LW(\u0027OVF properties config file was not found. The \u0027"},{"line_number":184,"context_line":"                         \"OVF config file is \u0027ovf-metadata.json\u0027. Please set \""},{"line_number":185,"context_line":"                         \"\u0027ovf_metadata_properties\u0027 to a list of interested \""},{"line_number":186,"context_line":"                         \u0027properties in the config file.\u0027))"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba15a1d1_b4aa4b6f","line":183,"updated":"2015-09-14 10:41:01.000000000","message":"Ditto","commit_id":"e772923632f989fa82b05342dd99d62ec519b730"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":20,"context_line":"except ImportError:"},{"line_number":21,"context_line":"    import xml.etree.ElementTree as ET"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import logging"},{"line_number":24,"context_line":"import os"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from oslo_config import cfg"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_21e6c2b4","line":23,"updated":"2015-09-15 14:43:13.000000000","message":"nit: this should probably be grouped with tarfile","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            else:"},{"line_number":78,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"},{"line_number":79,"context_line":"        else:"},{"line_number":80,"context_line":"            LOG.error(_LE(\"Expected \u0027ova\u0027 image container format for \""},{"line_number":81,"context_line":"                          \u0027OVF_Process task\u0027))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        return file_path"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_1c4e806f","line":80,"updated":"2015-09-15 14:43:13.000000000","message":"not an error, prob just debug since this will happen often for non-ova images.","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            LOG.error(_LE(\"Expected \u0027ova\u0027 image container format for \""},{"line_number":81,"context_line":"                          \u0027OVF_Process task\u0027))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        return file_path"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"class OVAImageExtractor(object):"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_21300201","line":83,"updated":"2015-09-15 14:43:13.000000000","message":"We also need to implement a \"revert\" method that checks if `$image_id.extracted` exists and deletes it.","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            filenames \u003d tar_file.getnames()"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"            ovf_filename \u003d next((filename for filename in filenames"},{"line_number":113,"context_line":"                                 if filename.endswith(\u0027.ovf\u0027)), None)"},{"line_number":114,"context_line":"            if ovf_filename:"},{"line_number":115,"context_line":"                ovf \u003d tar_file.extractfile(ovf_filename)"},{"line_number":116,"context_line":"                properties \u003d self._parse_OVF(ovf)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_f9b95e7a","line":113,"updated":"2015-09-15 14:43:13.000000000","message":"if there\u0027s no `ovf`, this should fail","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                ovf.close()"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"            for filename in filenames:"},{"line_number":120,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"},{"line_number":121,"context_line":"                    disk_name \u003d filename"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_8c1ca514","line":120,"updated":"2015-09-15 14:43:13.000000000","message":"In a previous patch-set, I mentioned we should used the OVF\u0027s metadata to know what disk to use. That\u0027d make it easier to recognize the main/boot disk.\n\nAs for the multiple disk case, I think it\u0027s ok to skip it for now but please, add a note here.","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":122,"context_line":""},{"line_number":123,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"},{"line_number":124,"context_line":"            if not disk:"},{"line_number":125,"context_line":"                LOG.error(_LE(\u0027Could not find valid disk image in OVA \u0027"},{"line_number":126,"context_line":"                              \u0027archive file\u0027))"},{"line_number":127,"context_line":"            return (disk, properties)"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_2c4c796f","line":125,"updated":"2015-09-15 14:43:13.000000000","message":"This should raise an error, not just log it. The image should be killed if this case is true.","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":176,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":177,"context_line":"                if not self.interested_properties:"},{"line_number":178,"context_line":"                    LOG.warn(_LW(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":179,"context_line":"                                 \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":180,"context_line":"                                 \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":181,"context_line":"                                 \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_f9a2be4a","line":178,"updated":"2015-09-15 14:43:13.000000000","message":"pls, address Erno\u0027s comments about logging levels","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":17561,"name":"Jakub Jasek","email":"jakub.jasek@tieto.com","username":"jjasek"},"change_message_id":"43cfcfc4631735b7e2108757e3051aaa15aa547c","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"},{"line_number":126,"context_line":"            if not disk:"},{"line_number":127,"context_line":"                raise RuntimeError(_(\u0027Could not find valid disk image in OVA \u0027"},{"line_number":128,"context_line":"                                     \u0027archive file\u0027))"},{"line_number":129,"context_line":"            return (disk, properties)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_60bc5106","line":127,"updated":"2015-09-17 12:17:38.000000000","message":"Death branch. Exception is already raised by tar_file.extractfile(disk_name) when disk_name is None","commit_id":"eb2e2be427f2d3f399fb20c52497b79116c19305"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"792a14aa3c77e350797423ca50905250e430b869","unresolved":false,"context_lines":[{"line_number":156,"context_line":"                    properties[tag] \u003d (elem.text.strip() if elem.text"},{"line_number":157,"context_line":"                                       else \u0027\u0027)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"                if tag \u003d\u003d \u0027References\u0027:"},{"line_number":160,"context_line":"                    files \u003d list(elem)"},{"line_number":161,"context_line":"                    if not len(files):"},{"line_number":162,"context_line":"                        raise RuntimeError(_(\u0027Could not find valid disk \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_8ba50b48","line":159,"updated":"2015-09-17 11:52:41.000000000","message":"I think this is not enough. I think we should get the \"disk\" from the \"Disk  section\" and use the fileId from there to get the exact file we\u0027re interested in from the `References` section.\n\nThat way we\u0027ll be sure it\u0027s a disk file (since it\u0027s in the Disk section) and that it\u0027s likely bootable (if there are several disks, I\u0027d probably pick the first in this case).\n\nhttp://www.dmtf.org/sites/default/files/standards/documents/DSP0243_2.1.0.pdf (Section 9.1 DiskSection)","commit_id":"eb2e2be427f2d3f399fb20c52497b79116c19305"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"792a14aa3c77e350797423ca50905250e430b869","unresolved":false,"context_lines":[{"line_number":171,"context_line":"                                                 \u0027file.\u0027))"},{"line_number":172,"context_line":"                        href \u003d file_element.get(href)"},{"line_number":173,"context_line":"                        if href.endswith(tuple("},{"line_number":174,"context_line":"                                CONF.image_format.disk_formats)):"},{"line_number":175,"context_line":"                            if disk_filename:"},{"line_number":176,"context_line":"                                raise RuntimeError(_(\u0027Multiple disk images \u0027"},{"line_number":177,"context_line":"                                                     \u0027are not supported.\u0027))"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_8b2aeb8e","line":174,"updated":"2015-09-17 11:52:41.000000000","message":"It really bothers me that we\u0027re checking the file extension to verify if we support it. AFAIK, the disk section has an `ovf:format` which should contain the format of the virtual disk. It\u0027d be better to use that section to verify the disk format rather than the file extension.\n\nhttp://www.dmtf.org/sites/default/files/standards/documents/DSP0243_2.1.0.pdf (see section 5.2 Virtual disk formats)","commit_id":"eb2e2be427f2d3f399fb20c52497b79116c19305"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"1bcb011abf7b48ee42bbb359d08adef49ec63010","unresolved":false,"context_lines":[{"line_number":171,"context_line":"                                                 \u0027file.\u0027))"},{"line_number":172,"context_line":"                        href \u003d file_element.get(href)"},{"line_number":173,"context_line":"                        if href.endswith(tuple("},{"line_number":174,"context_line":"                                CONF.image_format.disk_formats)):"},{"line_number":175,"context_line":"                            if disk_filename:"},{"line_number":176,"context_line":"                                raise RuntimeError(_(\u0027Multiple disk images \u0027"},{"line_number":177,"context_line":"                                                     \u0027are not supported.\u0027))"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_1d9dbebb","line":174,"in_reply_to":"ba15a1d1_8b2aeb8e","updated":"2015-09-17 11:54:37.000000000","message":"Eventually, we could have this done by the Introspect task, which already gets the metadata out of the image.","commit_id":"eb2e2be427f2d3f399fb20c52497b79116c19305"},{"author":{"_account_id":4992,"name":"malini-k-bhandaru","email":"malini.k.bhandaru@intel.com","username":"malini-k-bhandaru"},"change_message_id":"c50f8066e0e47c66f03784489922ad5a9906f8a9","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                        if href.endswith(tuple("},{"line_number":174,"context_line":"                                CONF.image_format.disk_formats)):"},{"line_number":175,"context_line":"                            if disk_filename:"},{"line_number":176,"context_line":"                                raise RuntimeError(_(\u0027Multiple disk images \u0027"},{"line_number":177,"context_line":"                                                     \u0027are not supported.\u0027))"},{"line_number":178,"context_line":"                            disk_filename \u003d href"},{"line_number":179,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_cedec010","line":176,"updated":"2015-09-17 05:24:12.000000000","message":"Do not raise exception, consume the first. Perhaps a warning message.","commit_id":"eb2e2be427f2d3f399fb20c52497b79116c19305"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"792a14aa3c77e350797423ca50905250e430b869","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                        if href.endswith(tuple("},{"line_number":174,"context_line":"                                CONF.image_format.disk_formats)):"},{"line_number":175,"context_line":"                            if disk_filename:"},{"line_number":176,"context_line":"                                raise RuntimeError(_(\u0027Multiple disk images \u0027"},{"line_number":177,"context_line":"                                                     \u0027are not supported.\u0027))"},{"line_number":178,"context_line":"                            disk_filename \u003d href"},{"line_number":179,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_8bf74b58","line":176,"in_reply_to":"ba15a1d1_cedec010","updated":"2015-09-17 11:52:41.000000000","message":"I\u0027d be a bit hesitant of using the first file, they may not be in the right order and we\u0027d attempt to boot a file that\u0027s not even bootable - correct me if I\u0027m wrong - since this code is picking files directly from the reference section w/o introspecting the Disk section.","commit_id":"eb2e2be427f2d3f399fb20c52497b79116c19305"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"b5825e4e12ca276d96e892d5b9310cf215b5f120","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":66,"context_line":"                data_iter \u003d script_utils.get_image_data_iter(file_path)"},{"line_number":67,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"},{"line_number":68,"context_line":"                image.extra_properties.update(properties)"},{"line_number":69,"context_line":"                self.image_repo.save(image)"},{"line_number":70,"context_line":"                dest_path \u003d os.path.join(CONF.task.work_dir,"},{"line_number":71,"context_line":"                                         \"%s.extracted\" % image_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_977365b4","line":68,"updated":"2015-09-21 19:31:39.000000000","message":"If the disk was extracted successfully, the resulting image is no longer an OVA. Thus, the container_format should be updated to \u0027bare\u0027.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"f1bacc06f28fdaa51f3c1b23167c2ac95b189dfa","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":66,"context_line":"                data_iter \u003d script_utils.get_image_data_iter(file_path)"},{"line_number":67,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"},{"line_number":68,"context_line":"                image.extra_properties.update(properties)"},{"line_number":69,"context_line":"                self.image_repo.save(image)"},{"line_number":70,"context_line":"                dest_path \u003d os.path.join(CONF.task.work_dir,"},{"line_number":71,"context_line":"                                         \"%s.extracted\" % image_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_8fed89b9","line":68,"in_reply_to":"ba15a1d1_977365b4","updated":"2015-09-21 22:35:28.000000000","message":"Thanks, Good catch :)","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"fc6fa742178a353c8771605673a03e5905f71b43","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                os.rename(dest_path, file_path.split(\"file://\")[-1])"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"            else:"},{"line_number":78,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        return file_path"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_06adf887","line":78,"updated":"2015-09-21 15:22:16.000000000","message":"Is this really worth a warn ? Erno?\n\nAsking because if users import ova files, those files will still be imported but this line will flood logs and it provides not useful information for OPs and certainly not for end-users.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"b5825e4e12ca276d96e892d5b9310cf215b5f120","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                os.rename(dest_path, file_path.split(\"file://\")[-1])"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"            else:"},{"line_number":78,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        return file_path"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_d7da3d17","line":78,"in_reply_to":"ba15a1d1_06adf887","updated":"2015-09-21 19:31:39.000000000","message":"This can be a DEBUG/INFO IMO.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c2b6a37f2d45bc063ec26a6f5a5f0d6449c53907","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                os.rename(dest_path, file_path.split(\"file://\")[-1])"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"            else:"},{"line_number":78,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        return file_path"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_d53c6ec1","line":78,"in_reply_to":"ba15a1d1_af264d14","updated":"2015-09-21 23:57:52.000000000","message":"Yes DEBUG, unaware user would not see the logs for warning anyways and info is just for positive unit of work. Consistency wise WARN would work (we do warn for some user error cases iirc), but current community push is that user errors should not be considered failures to operator.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"f1bacc06f28fdaa51f3c1b23167c2ac95b189dfa","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                os.rename(dest_path, file_path.split(\"file://\")[-1])"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"            else:"},{"line_number":78,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        return file_path"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_af264d14","line":78,"in_reply_to":"ba15a1d1_d7da3d17","updated":"2015-09-21 22:35:28.000000000","message":"Debug is good here. initially, thought to warn for unaware users.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"b5825e4e12ca276d96e892d5b9310cf215b5f120","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":199,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":200,"context_line":"                if not self.interested_properties:"},{"line_number":201,"context_line":"                    LOG.error(_LE(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":202,"context_line":"                                  \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_5725cd59","line":201,"updated":"2015-09-21 19:31:39.000000000","message":"We log an Error if the json is not found but let the task continue successfully (it just extracts the disk).\n\nIf this is Error, we should raise RuntimeError and fail the task IMO. @Erno, I remember you wanted this to be Error but I assume you also expected the task to fail, right ?","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c2b6a37f2d45bc063ec26a6f5a5f0d6449c53907","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":199,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":200,"context_line":"                if not self.interested_properties:"},{"line_number":201,"context_line":"                    LOG.error(_LE(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":202,"context_line":"                                  \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_d56d4e9a","line":201,"in_reply_to":"ba15a1d1_3a9599e3","updated":"2015-09-21 23:57:52.000000000","message":"Correct, my expectation would have been that if the task is not configured, it should error out and processing halt. If we are going to continue and will not fail any of the processing, then DEBUG is the right way to go. Again INFO should be just for positive units of work.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"b9263e1285355b8a55fb419357ccfa607322fa79","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":199,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":200,"context_line":"                if not self.interested_properties:"},{"line_number":201,"context_line":"                    LOG.error(_LE(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":202,"context_line":"                                  \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_35884a22","line":201,"in_reply_to":"ba15a1d1_3a9599e3","updated":"2015-09-21 23:33:49.000000000","message":"I agree. Seems there\u0027s two conficting comments here.\n@Erno, can you help us here? I do remember you wanted Error here.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"2d25bbddeb69e5bafdf7db7f4fb77969113abf09","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":199,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":200,"context_line":"                if not self.interested_properties:"},{"line_number":201,"context_line":"                    LOG.error(_LE(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":202,"context_line":"                                  \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_fa34014b","line":201,"in_reply_to":"ba15a1d1_5725cd59","updated":"2015-09-21 22:51:35.000000000","message":"I remember Flavio mentioned that users may set empty list if they don\u0027t want properties. Maybe not best to error here.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"3d8a5733400d1afedfe6097e6fd1a143699228dd","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":199,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":200,"context_line":"                if not self.interested_properties:"},{"line_number":201,"context_line":"                    LOG.error(_LE(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":202,"context_line":"                                  \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_e7e22901","line":201,"in_reply_to":"ba15a1d1_d56d4e9a","updated":"2015-09-22 00:10:37.000000000","message":"Thanks Erno. So DEBUG sounds good here for me too.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"f100b4162be618006725309ab96830d0f1cf10b7","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                self.interested_properties \u003d properties.get("},{"line_number":199,"context_line":"                    \u0027ovf_metadata_properties\u0027, [])"},{"line_number":200,"context_line":"                if not self.interested_properties:"},{"line_number":201,"context_line":"                    LOG.error(_LE(\u0027OVF metadata of interest was not specified \u0027"},{"line_number":202,"context_line":"                                  \u0027in ovf-metadata.json config file. Please \u0027"},{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_3a9599e3","line":201,"in_reply_to":"ba15a1d1_fa34014b","updated":"2015-09-21 23:03:24.000000000","message":"Yeah - so in that case shouldn\u0027t this be WARN or INFO ? If we log ERROR, it seems to me that the task has failed though it continues to execute here.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"f1bacc06f28fdaa51f3c1b23167c2ac95b189dfa","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"},{"line_number":205,"context_line":"        else:"},{"line_number":206,"context_line":"            LOG.error(_LE(\u0027OVF properties config file was not found. The \u0027"},{"line_number":207,"context_line":"                          \"OVF config file is \u0027ovf-metadata.json\u0027. Please set \""},{"line_number":208,"context_line":"                          \"\u0027ovf_metadata_properties\u0027 to a list of interested \""},{"line_number":209,"context_line":"                          \u0027properties in the config file.\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_7ab9d12d","line":206,"updated":"2015-09-21 22:35:28.000000000","message":"So let\u0027s raise error for no set properties and also no conf file?","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"79bff784275e382a2cd6671142ddee80ec10d45c","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"},{"line_number":205,"context_line":"        else:"},{"line_number":206,"context_line":"            LOG.error(_LE(\u0027OVF properties config file was not found. The \u0027"},{"line_number":207,"context_line":"                          \"OVF config file is \u0027ovf-metadata.json\u0027. Please set \""},{"line_number":208,"context_line":"                          \"\u0027ovf_metadata_properties\u0027 to a list of interested \""},{"line_number":209,"context_line":"                          \u0027properties in the config file.\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_9b62a4d7","line":206,"in_reply_to":"ba15a1d1_27aee198","updated":"2015-09-22 02:58:30.000000000","message":"Agreed. Debug sounds good here too.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"3d8a5733400d1afedfe6097e6fd1a143699228dd","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"},{"line_number":205,"context_line":"        else:"},{"line_number":206,"context_line":"            LOG.error(_LE(\u0027OVF properties config file was not found. The \u0027"},{"line_number":207,"context_line":"                          \"OVF config file is \u0027ovf-metadata.json\u0027. Please set \""},{"line_number":208,"context_line":"                          \"\u0027ovf_metadata_properties\u0027 to a list of interested \""},{"line_number":209,"context_line":"                          \u0027properties in the config file.\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_27aee198","line":206,"in_reply_to":"ba15a1d1_55085ed9","updated":"2015-09-22 00:10:37.000000000","message":"Guess this scenario is similar as L200 where we don\u0027t find any metadata from the file (just that the metadata json itself is missing here). \n\n@Kent: IMO, this metadata extraction can be considered an optional step for this task, so I am ok not raising any RuntimeError. But the log level should be changed to DEBUG if you are doing that.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c2b6a37f2d45bc063ec26a6f5a5f0d6449c53907","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"},{"line_number":205,"context_line":"        else:"},{"line_number":206,"context_line":"            LOG.error(_LE(\u0027OVF properties config file was not found. The \u0027"},{"line_number":207,"context_line":"                          \"OVF config file is \u0027ovf-metadata.json\u0027. Please set \""},{"line_number":208,"context_line":"                          \"\u0027ovf_metadata_properties\u0027 to a list of interested \""},{"line_number":209,"context_line":"                          \u0027properties in the config file.\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_55085ed9","line":206,"in_reply_to":"ba15a1d1_5a15ed98","updated":"2015-09-21 23:57:52.000000000","message":"I think if the conf file is fully missing (not just having empty list) we can error out and fail the task.","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"2d25bbddeb69e5bafdf7db7f4fb77969113abf09","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                                  \"set \u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":204,"context_line":"                                  \u0027interested properties\u0027))"},{"line_number":205,"context_line":"        else:"},{"line_number":206,"context_line":"            LOG.error(_LE(\u0027OVF properties config file was not found. The \u0027"},{"line_number":207,"context_line":"                          \"OVF config file is \u0027ovf-metadata.json\u0027. Please set \""},{"line_number":208,"context_line":"                          \"\u0027ovf_metadata_properties\u0027 to a list of interested \""},{"line_number":209,"context_line":"                          \u0027properties in the config file.\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba15a1d1_5a15ed98","line":206,"in_reply_to":"ba15a1d1_7ab9d12d","updated":"2015-09-21 22:51:35.000000000","message":"On second thought, maybe not good idea","commit_id":"321d80f46c3dd256ae93e7ebbc15e267c9546428"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"00334c0cf30982d91fdb293bb0ffc01390d5c56a","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        if image.container_format \u003d\u003d \u0027ova\u0027:"},{"line_number":70,"context_line":"            # FIXME(dramakri): This is an admin-only feature for security"},{"line_number":71,"context_line":"            # reasons. Ideally this should be achieved by making the import"},{"line_number":72,"context_line":"            # task API admin only. This is one of the items that the upcoming"},{"line_number":73,"context_line":"            # import refactoring work plans to do. Until then, we will check"},{"line_number":74,"context_line":"            # the context as a short-cut."},{"line_number":75,"context_line":"            if image.context and image.context.is_admin:"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a57f1b5_102b1aed","line":72,"updated":"2016-02-10 04:59:07.000000000","message":"This change has merged. https://review.openstack.org/#/c/265448/","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"00334c0cf30982d91fdb293bb0ffc01390d5c56a","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            # task API admin only. This is one of the items that the upcoming"},{"line_number":73,"context_line":"            # import refactoring work plans to do. Until then, we will check"},{"line_number":74,"context_line":"            # the context as a short-cut."},{"line_number":75,"context_line":"            if image.context and image.context.is_admin:"},{"line_number":76,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":77,"context_line":"                data_iter \u003d script_utils.get_image_data_iter(file_path)"},{"line_number":78,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a57f1b5_30309656","line":75,"updated":"2016-02-10 04:59:07.000000000","message":"Hence, this check can be removed.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"906095f4c3d7acea7957c87b4c57370f348e7b49","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            # task API admin only. This is one of the items that the upcoming"},{"line_number":73,"context_line":"            # import refactoring work plans to do. Until then, we will check"},{"line_number":74,"context_line":"            # the context as a short-cut."},{"line_number":75,"context_line":"            if image.context and image.context.is_admin:"},{"line_number":76,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":77,"context_line":"                data_iter \u003d script_utils.get_image_data_iter(file_path)"},{"line_number":78,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a57f1b5_c9094397","line":75,"in_reply_to":"3a57f1b5_30309656","updated":"2016-02-10 15:01:21.000000000","message":"Not yet. Ideally, even when this task will be used under the new import workflow, we shouldn\u0027t run it for images imported by users. Not yet, at least.\n\nI\u0027d prefer keeping it for now.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"6fb18bbbcefc494d7ec2b51de64e0da4f97c876f","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            # task API admin only. This is one of the items that the upcoming"},{"line_number":73,"context_line":"            # import refactoring work plans to do. Until then, we will check"},{"line_number":74,"context_line":"            # the context as a short-cut."},{"line_number":75,"context_line":"            if image.context and image.context.is_admin:"},{"line_number":76,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":77,"context_line":"                data_iter \u003d script_utils.get_image_data_iter(file_path)"},{"line_number":78,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a54edaf_43d1e6fa","line":75,"in_reply_to":"3a57f1b5_c9094397","updated":"2016-02-13 02:02:44.000000000","message":"Based on Flavio\u0027s comment, keeping the check as it is for now.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"00334c0cf30982d91fdb293bb0ffc01390d5c56a","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                dest_path \u003d os.path.join(CONF.task.work_dir,"},{"line_number":83,"context_line":"                                         \"%s.extracted\" % image_id)"},{"line_number":84,"context_line":"                with open(dest_path, \u0027wb\u0027) as f:"},{"line_number":85,"context_line":"                    f.write(disk.read())"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"                os.rename(dest_path, file_path.split(\"file://\")[-1])"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"3a57f1b5_b3680dbc","line":85,"updated":"2016-02-10 04:59:07.000000000","message":"This would read the entire extracted fileobj to the memory!\n\nYou should probably read/write in chunks.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"6fb18bbbcefc494d7ec2b51de64e0da4f97c876f","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                dest_path \u003d os.path.join(CONF.task.work_dir,"},{"line_number":83,"context_line":"                                         \"%s.extracted\" % image_id)"},{"line_number":84,"context_line":"                with open(dest_path, \u0027wb\u0027) as f:"},{"line_number":85,"context_line":"                    f.write(disk.read())"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"                os.rename(dest_path, file_path.split(\"file://\")[-1])"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"1a54edaf_63ce625c","line":85,"in_reply_to":"3a57f1b5_09475bbc","updated":"2016-02-13 02:02:44.000000000","message":"Done","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"906095f4c3d7acea7957c87b4c57370f348e7b49","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                dest_path \u003d os.path.join(CONF.task.work_dir,"},{"line_number":83,"context_line":"                                         \"%s.extracted\" % image_id)"},{"line_number":84,"context_line":"                with open(dest_path, \u0027wb\u0027) as f:"},{"line_number":85,"context_line":"                    f.write(disk.read())"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"                os.rename(dest_path, file_path.split(\"file://\")[-1])"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"3a57f1b5_09475bbc","line":85,"in_reply_to":"3a57f1b5_b3680dbc","updated":"2016-02-10 15:01:21.000000000","message":"good catch","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"00334c0cf30982d91fdb293bb0ffc01390d5c56a","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        disk_filename, file_elements, file_ref \u003d None, None, None"},{"line_number":159,"context_line":"        properties \u003d {}"},{"line_number":160,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":161,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":162,"context_line":"                tag \u003d _remove_qualified_namespace(elem.tag)"},{"line_number":163,"context_line":"                if tag in self.interested_properties:"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a57f1b5_f3d0c5cb","line":160,"updated":"2016-02-10 04:59:07.000000000","message":"IMO, I think it would be really simple to extract the disk_name if you do something like this:\nhttps://github.com/openstack/nova/blob/master/nova/virt/vmwareapi/images.py#L389","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"dd4ef369c364af11b13eb5903a8a4250e5577e7a","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        disk_filename, file_elements, file_ref \u003d None, None, None"},{"line_number":159,"context_line":"        properties \u003d {}"},{"line_number":160,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":161,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":162,"context_line":"                tag \u003d _remove_qualified_namespace(elem.tag)"},{"line_number":163,"context_line":"                if tag in self.interested_properties:"}],"source_content_type":"text/x-python","patch_set":23,"id":"dae33548_eb078615","line":160,"in_reply_to":"1a54edaf_23d86a1d","updated":"2016-02-18 23:33:00.000000000","message":"The same lookup methods can be used for extracting the interested property with something like :-\n\n for property in self.interested_propeties:\n  ovf.find(\"./%s%s\" % (pasd_ns, property)\n\nBut I think we can optimize later, I don\u0027t want to block this patch just for this.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"6fb18bbbcefc494d7ec2b51de64e0da4f97c876f","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        disk_filename, file_elements, file_ref \u003d None, None, None"},{"line_number":159,"context_line":"        properties \u003d {}"},{"line_number":160,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":161,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":162,"context_line":"                tag \u003d _remove_qualified_namespace(elem.tag)"},{"line_number":163,"context_line":"                if tag in self.interested_properties:"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a54edaf_23d86a1d","line":160,"in_reply_to":"3a57f1b5_f3d0c5cb","updated":"2016-02-13 02:02:44.000000000","message":"We have to iterate through the xml file entry-by-entry for the purpose of extracting the interested properties. It is simpler to handle disk_name as part of this iteration rather than use a completely different way of extracting just that one property.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"93a4f0773d1086776d37942406189cd749831cc1","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        disk_filename, file_elements, file_ref \u003d None, None, None"},{"line_number":159,"context_line":"        properties \u003d {}"},{"line_number":160,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":161,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":162,"context_line":"                tag \u003d _remove_qualified_namespace(elem.tag)"},{"line_number":163,"context_line":"                if tag in self.interested_properties:"}],"source_content_type":"text/x-python","patch_set":23,"id":"bae84128_bbd3ae5e","line":160,"in_reply_to":"dae33548_eb078615","updated":"2016-02-22 07:02:21.000000000","message":"Acknowledge.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"00334c0cf30982d91fdb293bb0ffc01390d5c56a","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":161,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":162,"context_line":"                tag \u003d _remove_qualified_namespace(elem.tag)"},{"line_number":163,"context_line":"                if tag in self.interested_properties:"},{"line_number":164,"context_line":"                    properties[tag] \u003d (elem.text.strip() if elem.text"},{"line_number":165,"context_line":"                                       else \u0027\u0027)"},{"line_number":166,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"3a57f1b5_33189d84","line":163,"updated":"2016-02-10 04:59:07.000000000","message":"In the spec, we decided to restrict extracting the properties to CIM_ProcessorAllocationSettingData tags. Can we explicitly do that ? Currenlty, you are matching any tag name.","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"6fb18bbbcefc494d7ec2b51de64e0da4f97c876f","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":161,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":162,"context_line":"                tag \u003d _remove_qualified_namespace(elem.tag)"},{"line_number":163,"context_line":"                if tag in self.interested_properties:"},{"line_number":164,"context_line":"                    properties[tag] \u003d (elem.text.strip() if elem.text"},{"line_number":165,"context_line":"                                       else \u0027\u0027)"},{"line_number":166,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"1a54edaf_c3039675","line":163,"in_reply_to":"3a57f1b5_33189d84","updated":"2016-02-13 02:02:44.000000000","message":"Done","commit_id":"dca4cd6c86d2ae72b3356df8fa0a044c4af95177"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":172,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":173,"context_line":"                ns, tag \u003d _get_namespace_and_tag(elem.tag)"},{"line_number":174,"context_line":"                if ns in CIM_NS:"},{"line_number":175,"context_line":"                    if tag in self.interested_properties:"},{"line_number":176,"context_line":"                        properties[CIM_NS[ns] + \u0027_\u0027 + tag] \u003d (elem.text.strip()"},{"line_number":177,"context_line":"                                                              if elem.text"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_d5134563","line":174,"updated":"2016-02-18 23:32:15.000000000","message":"This can be written without nesting if (just a nit)\n\n if CIM_NS.get(ns) and tag in self.interested_properties:","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":172,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":173,"context_line":"                ns, tag \u003d _get_namespace_and_tag(elem.tag)"},{"line_number":174,"context_line":"                if ns in CIM_NS:"},{"line_number":175,"context_line":"                    if tag in self.interested_properties:"},{"line_number":176,"context_line":"                        properties[CIM_NS[ns] + \u0027_\u0027 + tag] \u003d (elem.text.strip()"},{"line_number":177,"context_line":"                                                              if elem.text"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_75f5c7c9","line":174,"in_reply_to":"dae33548_d5134563","updated":"2016-02-22 06:53:44.000000000","message":"Done","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"}],"glance/common/config.py":[{"author":{"_account_id":6601,"name":"James Carey","email":"jecarey@yerac.com","username":"jecarey"},"change_message_id":"25b9ced57d6065140182981e7ba538c354826816","unresolved":false,"context_lines":[{"line_number":174,"context_line":"                      \u0027supported by the version of OpenSSL on the platform.\u0027"},{"line_number":175,"context_line":"                      \u0027 Examples are \"sha1\", \"sha256\", \"sha512\", etc.\u0027)),"},{"line_number":176,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003d[],"},{"line_number":177,"context_line":"                help\u003d_(\u0027Specifies which interested metadata should be \u0027"},{"line_number":178,"context_line":"                       \u0027extracted from an uploaded ovf file\u0027)),"},{"line_number":179,"context_line":"]"},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_8f22e2a1","line":177,"updated":"2015-08-20 19:38:47.000000000","message":"This might just be that I don\u0027t understand the terminology.  Should the word \u0027interested\u0027 be dropped?   What makes it \u0027interested\u0027 metadata?","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":174,"context_line":"                      \u0027supported by the version of OpenSSL on the platform.\u0027"},{"line_number":175,"context_line":"                      \u0027 Examples are \"sha1\", \"sha256\", \"sha512\", etc.\u0027)),"},{"line_number":176,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003d[],"},{"line_number":177,"context_line":"                help\u003d_(\u0027Specifies which interested metadata should be \u0027"},{"line_number":178,"context_line":"                       \u0027extracted from an uploaded ovf file\u0027)),"},{"line_number":179,"context_line":"]"},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_15dc6317","line":177,"in_reply_to":"fa1b9901_8f22e2a1","updated":"2015-08-21 20:57:30.000000000","message":"I agree, Thanks","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":6601,"name":"James Carey","email":"jecarey@yerac.com","username":"jecarey"},"change_message_id":"25b9ced57d6065140182981e7ba538c354826816","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                      \u0027 Examples are \"sha1\", \"sha256\", \"sha512\", etc.\u0027)),"},{"line_number":176,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003d[],"},{"line_number":177,"context_line":"                help\u003d_(\u0027Specifies which interested metadata should be \u0027"},{"line_number":178,"context_line":"                       \u0027extracted from an uploaded ovf file\u0027)),"},{"line_number":179,"context_line":"]"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_af50060c","line":178,"updated":"2015-08-20 19:38:47.000000000","message":"nit: Should \u0027ovf\u0027 be capitalized?","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                      \u0027 Examples are \"sha1\", \"sha256\", \"sha512\", etc.\u0027)),"},{"line_number":176,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003d[],"},{"line_number":177,"context_line":"                help\u003d_(\u0027Specifies which interested metadata should be \u0027"},{"line_number":178,"context_line":"                       \u0027extracted from an uploaded ovf file\u0027)),"},{"line_number":179,"context_line":"]"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_35e167d1","line":178,"in_reply_to":"fa1b9901_af50060c","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"aca34497488734bab39228e2b785c432b5a3ec2d","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                       \"image attribute\"),"},{"line_number":60,"context_line":"                deprecated_opts\u003d[cfg.DeprecatedOpt(\u0027disk_formats\u0027,"},{"line_number":61,"context_line":"                                                   group\u003d\u0027DEFAULT\u0027)]),"},{"line_number":62,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003dNone,"},{"line_number":63,"context_line":"                help\u003d_(\u0027Specifies which metadata should be extracted\u0027"},{"line_number":64,"context_line":"                       \u0027from an uploaded OVF file\u0027)),"},{"line_number":65,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_bfec7f90","line":62,"updated":"2015-08-26 16:26:09.000000000","message":"As mentioned in the spec review I\u0027d really prefer not to put this into the standard config file.","commit_id":"b0f9adefd084fcb9e2b9996843132e4afe5e0ef5"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9ba65ba9db60d55df662af048cfe932bb40915b","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                       \"image attribute\"),"},{"line_number":60,"context_line":"                deprecated_opts\u003d[cfg.DeprecatedOpt(\u0027disk_formats\u0027,"},{"line_number":61,"context_line":"                                                   group\u003d\u0027DEFAULT\u0027)]),"},{"line_number":62,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003dNone,"},{"line_number":63,"context_line":"                help\u003d_(\u0027Specifies which metadata should be extracted\u0027"},{"line_number":64,"context_line":"                       \u0027from an uploaded OVF file\u0027)),"},{"line_number":65,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_3747d44c","line":62,"updated":"2015-09-03 09:17:30.000000000","message":"I\u0027d personally prefer to have this in a separate section (or even a separate file) since it\u0027s very specific to OVF. As an example, we have a separate file for schemas, although I know yet-another-config-file is not the most ideal thing to have in a deployment.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"b6749e04772a0cee66b6905c48306ab1773351de","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                       \"image attribute\"),"},{"line_number":60,"context_line":"                deprecated_opts\u003d[cfg.DeprecatedOpt(\u0027disk_formats\u0027,"},{"line_number":61,"context_line":"                                                   group\u003d\u0027DEFAULT\u0027)]),"},{"line_number":62,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003dNone,"},{"line_number":63,"context_line":"                help\u003d_(\u0027Specifies which metadata should be extracted\u0027"},{"line_number":64,"context_line":"                       \u0027from an uploaded OVF file\u0027)),"},{"line_number":65,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_4678972b","line":62,"in_reply_to":"da20952f_3747d44c","updated":"2015-09-03 10:03:45.000000000","message":"Thanks for bringing this up again. I flagged this already in Ps2 that I\u0027d like to see this being similar way it\u0027s own file as the metadef definitions are. As these property descriptions might get long and messy to embed into the config file.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a3f344c7a4febc3f3b7a5c5affa30c3997122c58","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                       \"image attribute\"),"},{"line_number":60,"context_line":"                deprecated_opts\u003d[cfg.DeprecatedOpt(\u0027disk_formats\u0027,"},{"line_number":61,"context_line":"                                                   group\u003d\u0027DEFAULT\u0027)]),"},{"line_number":62,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003dNone,"},{"line_number":63,"context_line":"                help\u003d_(\u0027Specifies which metadata should be extracted\u0027"},{"line_number":64,"context_line":"                       \u0027from an uploaded OVF file\u0027)),"},{"line_number":65,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_37800bbf","line":62,"in_reply_to":"da20952f_4678972b","updated":"2015-09-04 02:03:14.000000000","message":"How would one go about this implementation wise? I\u0027m unsure of all the steps it requires to register/load in a new config file. Been digging through the code but hitting many roadblocks (feels like there\u0027s a lot of required steps here) Thanks","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9ba65ba9db60d55df662af048cfe932bb40915b","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                deprecated_opts\u003d[cfg.DeprecatedOpt(\u0027disk_formats\u0027,"},{"line_number":61,"context_line":"                                                   group\u003d\u0027DEFAULT\u0027)]),"},{"line_number":62,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003dNone,"},{"line_number":63,"context_line":"                help\u003d_(\u0027Specifies which metadata should be extracted\u0027"},{"line_number":64,"context_line":"                       \u0027from an uploaded OVF file\u0027)),"},{"line_number":65,"context_line":"]"},{"line_number":66,"context_line":"task_opts \u003d ["}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_77ab0cd7","line":63,"updated":"2015-09-03 09:17:30.000000000","message":"Based on the spec, I believe this option was supposed to be a key\u003dvalue type. I might be wrong, though. It\u0027d be nice to have examples of common options in the help text.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a3f344c7a4febc3f3b7a5c5affa30c3997122c58","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                deprecated_opts\u003d[cfg.DeprecatedOpt(\u0027disk_formats\u0027,"},{"line_number":61,"context_line":"                                                   group\u003d\u0027DEFAULT\u0027)]),"},{"line_number":62,"context_line":"    cfg.ListOpt(\u0027ovf_metadata_properties\u0027, default\u003dNone,"},{"line_number":63,"context_line":"                help\u003d_(\u0027Specifies which metadata should be extracted\u0027"},{"line_number":64,"context_line":"                       \u0027from an uploaded OVF file\u0027)),"},{"line_number":65,"context_line":"]"},{"line_number":66,"context_line":"task_opts \u003d ["}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_d7718f0d","line":63,"in_reply_to":"da20952f_77ab0cd7","updated":"2015-09-04 02:03:14.000000000","message":"Yes It would just be one key (ovf_metadata_properties) with specified values:\ne.g.\novf_metadata_properties \u003d ProcessorArchitecture, InstructionSet, InstructionSetExtensionName","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"}],"glance/common/scripts/image_import/main.py":[{"author":{"_account_id":6601,"name":"James Carey","email":"jecarey@yerac.com","username":"jecarey"},"change_message_id":"25b9ced57d6065140182981e7ba538c354826816","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"CONF \u003d cfg.CONF"},{"line_number":47,"context_line":"CONF.import_opt(\u0027disk_formats\u0027, \u0027glance.common.config\u0027, group\u003d\u0027image_format\u0027)"},{"line_number":48,"context_line":"CONF.import_opt(\u0027ovf_metadata_properties\u0027, \u0027glance.common.config\u0027)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def run(t_id, context, task_repo, image_repo, image_factory):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_8fdf4239","line":48,"updated":"2015-08-20 19:38:47.000000000","message":"Don\u0027t these need to be added to glance/opts.py to the appropriate opts list (possibly _api_opts?) so they will get into the sample conf file?","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"CONF \u003d cfg.CONF"},{"line_number":47,"context_line":"CONF.import_opt(\u0027disk_formats\u0027, \u0027glance.common.config\u0027, group\u003d\u0027image_format\u0027)"},{"line_number":48,"context_line":"CONF.import_opt(\u0027ovf_metadata_properties\u0027, \u0027glance.common.config\u0027)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def run(t_id, context, task_repo, image_repo, image_factory):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_55dadb18","line":48,"in_reply_to":"fa1b9901_8fdf4239","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"301f01e7a76843ed38a1add86412f04708b013a1","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":161,"context_line":"    class OVAImageExtractor(object):"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        A class that extracts the disk image and ovf file from an OVA"},{"line_number":164,"context_line":"        tarball. Parses the OVF file for metadata of interest."},{"line_number":165,"context_line":"        Allows upload of disk image to Glance with included metadata."}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_5c7ee619","line":162,"updated":"2015-08-20 10:15:41.000000000","message":"multi line docstring is not as per standard,\n\nplease refer, http://docs.openstack.org/developer/hacking/#docstrings","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":161,"context_line":"    class OVAImageExtractor(object):"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        A class that extracts the disk image and ovf file from an OVA"},{"line_number":164,"context_line":"        tarball. Parses the OVF file for metadata of interest."},{"line_number":165,"context_line":"        Allows upload of disk image to Glance with included metadata."}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_35088791","line":162,"in_reply_to":"fa1b9901_5c7ee619","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":6601,"name":"James Carey","email":"jecarey@yerac.com","username":"jecarey"},"change_message_id":"25b9ced57d6065140182981e7ba538c354826816","unresolved":false,"context_lines":[{"line_number":160,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":161,"context_line":"    class OVAImageExtractor(object):"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        A class that extracts the disk image and ovf file from an OVA"},{"line_number":164,"context_line":"        tarball. Parses the OVF file for metadata of interest."},{"line_number":165,"context_line":"        Allows upload of disk image to Glance with included metadata."},{"line_number":166,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_0f46d24e","line":163,"updated":"2015-08-20 19:38:47.000000000","message":"nit: In all the comments and messages the capitalization of \u0027ovf\u0027 should be consistent as either \u0027ovf\u0027 or \u0027OVF\u0027 in the comments.  ditto for \u0027ova\u0027","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":160,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":161,"context_line":"    class OVAImageExtractor(object):"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        A class that extracts the disk image and ovf file from an OVA"},{"line_number":164,"context_line":"        tarball. Parses the OVF file for metadata of interest."},{"line_number":165,"context_line":"        Allows upload of disk image to Glance with included metadata."},{"line_number":166,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_f501af6b","line":163,"in_reply_to":"fa1b9901_0f46d24e","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":6601,"name":"James Carey","email":"jecarey@yerac.com","username":"jecarey"},"change_message_id":"25b9ced57d6065140182981e7ba538c354826816","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        def __init__(self):"},{"line_number":168,"context_line":"            self.interested_properties \u003d CONF.ovf_metadata_properties"},{"line_number":169,"context_line":"            if not self.interested_properties:"},{"line_number":170,"context_line":"                LOG.warn(_(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":171,"context_line":"                           \u0027Glance conf file. Please set \u0027"},{"line_number":172,"context_line":"                           \"\u0027ovf_metadata_properties\u0027 to a list of \""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_4f6cca34","line":169,"updated":"2015-08-20 19:38:47.000000000","message":"This will be true if the property is not set (default []) or if it was set with no values.   So, if they customer doesn\u0027t want any, there is no way to specify that in the configuration without it being interpreted that they didn\u0027t consider it.\n\nIs that OK?  If not, you could consider making the default None so it can discern between the two cases.","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        \"\"\""},{"line_number":167,"context_line":"        def __init__(self):"},{"line_number":168,"context_line":"            self.interested_properties \u003d CONF.ovf_metadata_properties"},{"line_number":169,"context_line":"            if not self.interested_properties:"},{"line_number":170,"context_line":"                LOG.warn(_(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":171,"context_line":"                           \u0027Glance conf file. Please set \u0027"},{"line_number":172,"context_line":"                           \"\u0027ovf_metadata_properties\u0027 to a list of \""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_f50f6f86","line":169,"in_reply_to":"fa1b9901_4f6cca34","updated":"2015-08-21 20:57:30.000000000","message":"Thanks, users can set empty list in conf file. default None sounds good","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"301f01e7a76843ed38a1add86412f04708b013a1","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        def __init__(self):"},{"line_number":168,"context_line":"            self.interested_properties \u003d CONF.ovf_metadata_properties"},{"line_number":169,"context_line":"            if not self.interested_properties:"},{"line_number":170,"context_line":"                LOG.warn(_(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":171,"context_line":"                           \u0027Glance conf file. Please set \u0027"},{"line_number":172,"context_line":"                           \"\u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":173,"context_line":"                           \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_fca7baba","line":170,"updated":"2015-08-20 10:15:41.000000000","message":"need to use _LW here","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        def __init__(self):"},{"line_number":168,"context_line":"            self.interested_properties \u003d CONF.ovf_metadata_properties"},{"line_number":169,"context_line":"            if not self.interested_properties:"},{"line_number":170,"context_line":"                LOG.warn(_(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":171,"context_line":"                           \u0027Glance conf file. Please set \u0027"},{"line_number":172,"context_line":"                           \"\u0027ovf_metadata_properties\u0027 to a list of \""},{"line_number":173,"context_line":"                           \u0027interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_956cf337","line":170,"in_reply_to":"fa1b9901_fca7baba","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"301f01e7a76843ed38a1add86412f04708b013a1","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                           \u0027interested properties\u0027))"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        def extract(self, ova):"},{"line_number":176,"context_line":"            \"\"\""},{"line_number":177,"context_line":"            Extracts a single disk image and ovf from ova tarball and calls"},{"line_number":178,"context_line":"            ovf parser method"},{"line_number":179,"context_line":"            :param ova: an ova file object"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_3c71b244","line":176,"updated":"2015-08-20 10:15:41.000000000","message":"ditto","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                           \u0027interested properties\u0027))"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        def extract(self, ova):"},{"line_number":176,"context_line":"            \"\"\""},{"line_number":177,"context_line":"            Extracts a single disk image and ovf from ova tarball and calls"},{"line_number":178,"context_line":"            ovf parser method"},{"line_number":179,"context_line":"            :param ova: an ova file object"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_9553936c","line":176,"in_reply_to":"fa1b9901_3c71b244","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":6601,"name":"James Carey","email":"jecarey@yerac.com","username":"jecarey"},"change_message_id":"25b9ced57d6065140182981e7ba538c354826816","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                        disk_name \u003d filename"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                disk \u003d tar_file.extractfile(disk_name)"},{"line_number":200,"context_line":"                if not disk:"},{"line_number":201,"context_line":"                    msg \u003d _(\u0027Could not find valid disk image in OVA package\u0027)"},{"line_number":202,"context_line":"                    LOG.error(msg)"},{"line_number":203,"context_line":"                return (disk, properties)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_0dc49a3d","line":200,"updated":"2015-08-20 19:38:47.000000000","message":"It seems like this should raise an exception and stop processing rather than just logging and going on.","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                        disk_name \u003d filename"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                disk \u003d tar_file.extractfile(disk_name)"},{"line_number":200,"context_line":"                if not disk:"},{"line_number":201,"context_line":"                    msg \u003d _(\u0027Could not find valid disk image in OVA package\u0027)"},{"line_number":202,"context_line":"                    LOG.error(msg)"},{"line_number":203,"context_line":"                return (disk, properties)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_351c0777","line":200,"in_reply_to":"fa1b9901_0dc49a3d","updated":"2015-08-21 20:57:30.000000000","message":"Wasn\u0027t sure about this part. Should it throw an error or just fall through and upload the ova archive.","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"301f01e7a76843ed38a1add86412f04708b013a1","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                disk \u003d tar_file.extractfile(disk_name)"},{"line_number":200,"context_line":"                if not disk:"},{"line_number":201,"context_line":"                    msg \u003d _(\u0027Could not find valid disk image in OVA package\u0027)"},{"line_number":202,"context_line":"                    LOG.error(msg)"},{"line_number":203,"context_line":"                return (disk, properties)"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_1c470ebd","line":201,"updated":"2015-08-20 10:15:41.000000000","message":"Need to use _LE here","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                disk \u003d tar_file.extractfile(disk_name)"},{"line_number":200,"context_line":"                if not disk:"},{"line_number":201,"context_line":"                    msg \u003d _(\u0027Could not find valid disk image in OVA package\u0027)"},{"line_number":202,"context_line":"                    LOG.error(msg)"},{"line_number":203,"context_line":"                return (disk, properties)"},{"line_number":204,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_b5501760","line":201,"in_reply_to":"fa1b9901_1c470ebd","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":6601,"name":"James Carey","email":"jecarey@yerac.com","username":"jecarey"},"change_message_id":"25b9ced57d6065140182981e7ba538c354826816","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                disk \u003d tar_file.extractfile(disk_name)"},{"line_number":200,"context_line":"                if not disk:"},{"line_number":201,"context_line":"                    msg \u003d _(\u0027Could not find valid disk image in OVA package\u0027)"},{"line_number":202,"context_line":"                    LOG.error(msg)"},{"line_number":203,"context_line":"                return (disk, properties)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        def _parse_OVF(self, ovf):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_6d4af627","line":202,"updated":"2015-08-20 19:38:47.000000000","message":"since msg is only used in the log, I\u0027d recommend using the string directly instead.","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                disk \u003d tar_file.extractfile(disk_name)"},{"line_number":200,"context_line":"                if not disk:"},{"line_number":201,"context_line":"                    msg \u003d _(\u0027Could not find valid disk image in OVA package\u0027)"},{"line_number":202,"context_line":"                    LOG.error(msg)"},{"line_number":203,"context_line":"                return (disk, properties)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        def _parse_OVF(self, ovf):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_554dfbc2","line":202,"in_reply_to":"fa1b9901_6d4af627","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"301f01e7a76843ed38a1add86412f04708b013a1","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                return (disk, properties)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        def _parse_OVF(self, ovf):"},{"line_number":206,"context_line":"            \"\"\""},{"line_number":207,"context_line":"            Parses the OVF file"},{"line_number":208,"context_line":"            Qualified namespaces are removed from the included properties."},{"line_number":209,"context_line":"            :param ovf: a file object containing the ovf file"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_fc181ad5","line":206,"updated":"2015-08-20 10:15:41.000000000","message":"ditto","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"7f49f5629384ec6924c34fb8130c3b5a517c80d9","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                return (disk, properties)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        def _parse_OVF(self, ovf):"},{"line_number":206,"context_line":"            \"\"\""},{"line_number":207,"context_line":"            Parses the OVF file"},{"line_number":208,"context_line":"            Qualified namespaces are removed from the included properties."},{"line_number":209,"context_line":"            :param ovf: a file object containing the ovf file"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_754a7fae","line":206,"in_reply_to":"fa1b9901_fc181ad5","updated":"2015-08-21 20:57:30.000000000","message":"Done","commit_id":"0ce78009f33ed83c6e98aebecf8cd16a8aef938e"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3834100231eed8d1d19f254751bada9e4493ac24","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":162,"context_line":"    class OVAImageExtractor(object):"},{"line_number":163,"context_line":"        \"\"\"Extracts and parses uploaded OVA tar file"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        A class that extracts the disk image and OVF file from an OVA"}],"source_content_type":"text/x-python","patch_set":2,"id":"da20952f_02799093","line":162,"updated":"2015-08-26 16:30:27.000000000","message":"Please don\u0027t do this within the set_image_data function. Makes the func really long and messy. I wouldn\u0027t mind ever if the OVAImageExtractor class became it\u0027s own module under scripts/image_import","commit_id":"b0f9adefd084fcb9e2b9996843132e4afe5e0ef5"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"93804568882749f099ce359f675e716ee7ab0974","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":162,"context_line":"    class OVAImageExtractor(object):"},{"line_number":163,"context_line":"        \"\"\"Extracts and parses uploaded OVA tar file"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        A class that extracts the disk image and OVF file from an OVA"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_703123a2","line":162,"updated":"2015-08-27 01:10:41.000000000","message":"Maybe just me, but seems strange to implement the class inside the function.","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":162,"context_line":"    class OVAImageExtractor(object):"},{"line_number":163,"context_line":"        \"\"\"Extracts and parses uploaded OVA tar file"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        A class that extracts the disk image and OVF file from an OVA"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_41db7d4f","line":162,"in_reply_to":"da20952f_305f2b74","updated":"2015-08-28 22:26:11.000000000","message":"Done","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":162,"context_line":"    class OVAImageExtractor(object):"},{"line_number":163,"context_line":"        \"\"\"Extracts and parses uploaded OVA tar file"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        A class that extracts the disk image and OVF file from an OVA"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_612321a8","line":162,"in_reply_to":"da20952f_703123a2","updated":"2015-08-28 22:26:11.000000000","message":"Done","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"0895f6c759e9119f0c0a3e699e912ac3fc8b4936","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def set_image_data(image, uri, task_id):"},{"line_number":162,"context_line":"    class OVAImageExtractor(object):"},{"line_number":163,"context_line":"        \"\"\"Extracts and parses uploaded OVA tar file"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        A class that extracts the disk image and OVF file from an OVA"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_305f2b74","line":162,"in_reply_to":"da20952f_703123a2","updated":"2015-08-27 01:19:51.000000000","message":"Unit testing will be easier if this class is outside of the function.","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"0895f6c759e9119f0c0a3e699e912ac3fc8b4936","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                             \"\u0027ovf_metadata_properties\u0027 under \u0027image_format\u0027 \""},{"line_number":177,"context_line":"                             \u0027group to a list of interested properties\u0027))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        def extract(self, ova):"},{"line_number":180,"context_line":"            \"\"\"Extracts disk image and OVF from OVA tar file"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"            Extracts a single disk image and OVF from OVA tar file and calls"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_d06d8f33","line":179,"updated":"2015-08-27 01:19:51.000000000","message":"Need unittest for this. I think unittests should execute the function directly not indirectly.","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                             \"\u0027ovf_metadata_properties\u0027 under \u0027image_format\u0027 \""},{"line_number":177,"context_line":"                             \u0027group to a list of interested properties\u0027))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        def extract(self, ova):"},{"line_number":180,"context_line":"            \"\"\"Extracts disk image and OVF from OVA tar file"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"            Extracts a single disk image and OVF from OVA tar file and calls"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_61b7a1f5","line":179,"in_reply_to":"da20952f_d06d8f33","updated":"2015-08-28 22:26:11.000000000","message":"Yes I think Jakub will do this.","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e6770db1dbab5a2ffc3653f093660bc96a5dc22f","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                disk_name, properties \u003d None, None"},{"line_number":190,"context_line":"                filenames \u003d tar_file.getnames()"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"                ovf_filename \u003d next((filename for filename in filenames"},{"line_number":193,"context_line":"                                     if \u0027.ovf\u0027 in filename), None)"},{"line_number":194,"context_line":"                if ovf_filename:"},{"line_number":195,"context_line":"                    ovf \u003d tar_file.extractfile(ovf_filename)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_9fb6403c","line":192,"updated":"2015-08-27 03:50:40.000000000","message":"What is the expected filename format searching for.  Would be good to document it.\n\nAt the moment this would match: not-an-ovf-file.txt","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"93804568882749f099ce359f675e716ee7ab0974","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                disk_name, properties \u003d None, None"},{"line_number":190,"context_line":"                filenames \u003d tar_file.getnames()"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"                ovf_filename \u003d next((filename for filename in filenames"},{"line_number":193,"context_line":"                                     if \u0027.ovf\u0027 in filename), None)"},{"line_number":194,"context_line":"                if ovf_filename:"},{"line_number":195,"context_line":"                    ovf \u003d tar_file.extractfile(ovf_filename)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_f0cbf370","line":192,"updated":"2015-08-27 01:10:41.000000000","message":"Why not use list comprehension?","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                disk_name, properties \u003d None, None"},{"line_number":190,"context_line":"                filenames \u003d tar_file.getnames()"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"                ovf_filename \u003d next((filename for filename in filenames"},{"line_number":193,"context_line":"                                     if \u0027.ovf\u0027 in filename), None)"},{"line_number":194,"context_line":"                if ovf_filename:"},{"line_number":195,"context_line":"                    ovf \u003d tar_file.extractfile(ovf_filename)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_21d1a9fa","line":192,"in_reply_to":"da20952f_9fb6403c","updated":"2015-08-28 22:26:11.000000000","message":"Yes filename format is \u0027name.ovf\u0027","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"93804568882749f099ce359f675e716ee7ab0974","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                filenames \u003d tar_file.getnames()"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"                ovf_filename \u003d next((filename for filename in filenames"},{"line_number":193,"context_line":"                                     if \u0027.ovf\u0027 in filename), None)"},{"line_number":194,"context_line":"                if ovf_filename:"},{"line_number":195,"context_line":"                    ovf \u003d tar_file.extractfile(ovf_filename)"},{"line_number":196,"context_line":"                    properties \u003d self._parse_OVF(ovf)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_f0e4530e","line":193,"updated":"2015-08-27 01:10:41.000000000","message":"Could this be:\n if filename.endswith(\u0027.ovf\u0027) ?","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                filenames \u003d tar_file.getnames()"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"                ovf_filename \u003d next((filename for filename in filenames"},{"line_number":193,"context_line":"                                     if \u0027.ovf\u0027 in filename), None)"},{"line_number":194,"context_line":"                if ovf_filename:"},{"line_number":195,"context_line":"                    ovf \u003d tar_file.extractfile(ovf_filename)"},{"line_number":196,"context_line":"                    properties \u003d self._parse_OVF(ovf)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_e19ad16d","line":193,"in_reply_to":"da20952f_f0e4530e","updated":"2015-08-28 22:26:11.000000000","message":"Good idea. Thanks!","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e6770db1dbab5a2ffc3653f093660bc96a5dc22f","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                    ovf.close()"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                for filename in filenames:"},{"line_number":200,"context_line":"                    if any(disk_format in filename for disk_format in"},{"line_number":201,"context_line":"                           CONF.image_format.disk_formats):"},{"line_number":202,"context_line":"                        disk_name \u003d filename"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_ff1f3c59","line":200,"updated":"2015-08-27 03:50:40.000000000","message":"This should work and seems simpler but I am unsure what the filename format is expected to be.  My assumption is that looking at extension.\n if filename.endswith(tuple(CONF.image_format.disk_formats)):\n\nAlso current way would trigger on file of name:\n   vmdk.exe or qcow2.exe","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                    ovf.close()"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                for filename in filenames:"},{"line_number":200,"context_line":"                    if any(disk_format in filename for disk_format in"},{"line_number":201,"context_line":"                           CONF.image_format.disk_formats):"},{"line_number":202,"context_line":"                        disk_name \u003d filename"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_01cd8565","line":200,"in_reply_to":"da20952f_ff1f3c59","updated":"2015-08-28 22:26:11.000000000","message":"Thanks","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"0895f6c759e9119f0c0a3e699e912ac3fc8b4936","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                                  \u0027archive file\u0027))"},{"line_number":208,"context_line":"                return (disk, properties)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        def _parse_OVF(self, ovf):"},{"line_number":211,"context_line":"            \"\"\"Parses the OVF file"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"            Parses the OVF file for specified metadata properties. Properties"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_90771702","line":210,"updated":"2015-08-27 01:19:51.000000000","message":"Need unittest for this. I think unittests should execute the function directly not indirectly.","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                                  \u0027archive file\u0027))"},{"line_number":208,"context_line":"                return (disk, properties)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        def _parse_OVF(self, ovf):"},{"line_number":211,"context_line":"            \"\"\"Parses the OVF file"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"            Parses the OVF file for specified metadata properties. Properties"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_81919549","line":210,"in_reply_to":"da20952f_90771702","updated":"2015-08-28 22:26:11.000000000","message":"Yes thanks","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"93804568882749f099ce359f675e716ee7ab0974","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            properties \u003d {}"},{"line_number":225,"context_line":"            for event, elem in ET.iterparse(ovf):"},{"line_number":226,"context_line":"                if event \u003d\u003d \u0027end\u0027:"},{"line_number":227,"context_line":"                    tag \u003d (elem.tag.split(\u0027}\u0027, 1)[1] if \u0027}\u0027 in elem.tag"},{"line_number":228,"context_line":"                           else elem.tag)"},{"line_number":229,"context_line":"                    if tag in self.interested_properties:"},{"line_number":230,"context_line":"                        properties[tag] \u003d (elem.text.strip() if elem.text"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_50a17fa8","line":227,"updated":"2015-08-27 01:10:41.000000000","message":"What is this doing?","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"75e9206ef74f58e90648569daf3a681325de9c88","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            properties \u003d {}"},{"line_number":225,"context_line":"            for event, elem in ET.iterparse(ovf):"},{"line_number":226,"context_line":"                if event \u003d\u003d \u0027end\u0027:"},{"line_number":227,"context_line":"                    tag \u003d (elem.tag.split(\u0027}\u0027, 1)[1] if \u0027}\u0027 in elem.tag"},{"line_number":228,"context_line":"                           else elem.tag)"},{"line_number":229,"context_line":"                    if tag in self.interested_properties:"},{"line_number":230,"context_line":"                        properties[tag] \u003d (elem.text.strip() if elem.text"}],"source_content_type":"text/x-python","patch_set":3,"id":"da20952f_61b321ea","line":227,"in_reply_to":"da20952f_50a17fa8","updated":"2015-08-28 22:26:11.000000000","message":"This is to remove the namespace. e.g. {\u003cnamespace\u003e}Item becomes Item","commit_id":"ae9876c5a611203b517f5286dc6df3f464a1ac44"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"d2df44098ac1969db907aec91c1ddf2487d8853e","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        properties \u003d {}"},{"line_number":252,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":253,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":254,"context_line":"                tag \u003d (elem.tag.split(\u0027}\u0027, 1)[1] if \u0027}\u0027 in elem.tag"},{"line_number":255,"context_line":"                       else elem.tag)"},{"line_number":256,"context_line":"                if tag in self.interested_properties:"},{"line_number":257,"context_line":"                    properties[tag] \u003d (elem.text.strip() if elem.text"}],"source_content_type":"text/x-python","patch_set":4,"id":"da20952f_5ca844db","line":254,"updated":"2015-08-28 22:33:12.000000000","message":"Can this be documented what is going on here? \n\nMaybe the XML library can do this?","commit_id":"1aab3fd1143e60cd141d06136ed4e89cbdb65521"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"acc93d3cfdcd2695d01104daaac0d8210982dcb1","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        properties \u003d {}"},{"line_number":252,"context_line":"        for event, elem in ET.iterparse(ovf):"},{"line_number":253,"context_line":"            if event \u003d\u003d \u0027end\u0027:"},{"line_number":254,"context_line":"                tag \u003d (elem.tag.split(\u0027}\u0027, 1)[1] if \u0027}\u0027 in elem.tag"},{"line_number":255,"context_line":"                       else elem.tag)"},{"line_number":256,"context_line":"                if tag in self.interested_properties:"},{"line_number":257,"context_line":"                    properties[tag] \u003d (elem.text.strip() if elem.text"}],"source_content_type":"text/x-python","patch_set":4,"id":"da20952f_dcfe94ad","line":254,"in_reply_to":"da20952f_5ca844db","updated":"2015-08-28 22:39:22.000000000","message":"I will comment this","commit_id":"1aab3fd1143e60cd141d06136ed4e89cbdb65521"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e41bd18a56f87108d637e6f37735663d9aa4faae","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from oslo_utils import encodeutils"},{"line_number":24,"context_line":"from oslo_utils import excutils"},{"line_number":25,"context_line":"import six"},{"line_number":26,"context_line":"import tarfile"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"try:"},{"line_number":29,"context_line":"    import xml.etree.cElementTree as ET"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_8bebeb12","line":26,"updated":"2015-08-31 06:23:45.000000000","message":"nit: tarfile is a built-in library. So should be above this section\n\n import tarfile\n\n from oslo_concurrency import lockutils\n ....","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a136c04fb32ec906ce7eea41dbd61eee2bdd91c5","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from oslo_utils import encodeutils"},{"line_number":24,"context_line":"from oslo_utils import excutils"},{"line_number":25,"context_line":"import six"},{"line_number":26,"context_line":"import tarfile"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"try:"},{"line_number":29,"context_line":"    import xml.etree.cElementTree as ET"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_f65d307a","line":26,"in_reply_to":"da20952f_8bebeb12","updated":"2015-09-01 20:35:49.000000000","message":"Done","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e41bd18a56f87108d637e6f37735663d9aa4faae","unresolved":false,"context_lines":[{"line_number":25,"context_line":"import six"},{"line_number":26,"context_line":"import tarfile"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"try:"},{"line_number":29,"context_line":"    import xml.etree.cElementTree as ET"},{"line_number":30,"context_line":"except ImportError:"},{"line_number":31,"context_line":"    import xml.etree.ElementTree as ET"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_6b44571a","line":28,"updated":"2015-08-31 06:23:45.000000000","message":"nit: This also should probably be above.","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a136c04fb32ec906ce7eea41dbd61eee2bdd91c5","unresolved":false,"context_lines":[{"line_number":25,"context_line":"import six"},{"line_number":26,"context_line":"import tarfile"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"try:"},{"line_number":29,"context_line":"    import xml.etree.cElementTree as ET"},{"line_number":30,"context_line":"except ImportError:"},{"line_number":31,"context_line":"    import xml.etree.ElementTree as ET"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_9653946c","line":28,"in_reply_to":"da20952f_6b44571a","updated":"2015-09-01 20:35:49.000000000","message":"Done","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e41bd18a56f87108d637e6f37735663d9aa4faae","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    \"\"\""},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def __init__(self):"},{"line_number":198,"context_line":"        self.interested_properties \u003d \\"},{"line_number":199,"context_line":"            CONF.image_format.ovf_metadata_properties"},{"line_number":200,"context_line":"        if self.interested_properties \u003d\u003d None:"},{"line_number":201,"context_line":"            LOG.warn(_LW(\u0027Interested OVF metadata was not specified in \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_0b4efb3a","line":198,"updated":"2015-08-31 06:23:45.000000000","message":"nit: Usually try to avoid backslash\n\nAlso: I believe that this will fit on one line as it is about 70 characters.\n\nIf too long for one line can do:\n  self.interested_properties \u003d (\n    CONF.image_format.ovf_metadata_properties)","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a136c04fb32ec906ce7eea41dbd61eee2bdd91c5","unresolved":false,"context_lines":[{"line_number":195,"context_line":"    \"\"\""},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def __init__(self):"},{"line_number":198,"context_line":"        self.interested_properties \u003d \\"},{"line_number":199,"context_line":"            CONF.image_format.ovf_metadata_properties"},{"line_number":200,"context_line":"        if self.interested_properties \u003d\u003d None:"},{"line_number":201,"context_line":"            LOG.warn(_LW(\u0027Interested OVF metadata was not specified in \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_9671547a","line":198,"in_reply_to":"da20952f_0b4efb3a","updated":"2015-09-01 20:35:49.000000000","message":"Done","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e41bd18a56f87108d637e6f37735663d9aa4faae","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    def __init__(self):"},{"line_number":198,"context_line":"        self.interested_properties \u003d \\"},{"line_number":199,"context_line":"            CONF.image_format.ovf_metadata_properties"},{"line_number":200,"context_line":"        if self.interested_properties \u003d\u003d None:"},{"line_number":201,"context_line":"            LOG.warn(_LW(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":202,"context_line":"                         \u0027Glance conf file. Please set \u0027"},{"line_number":203,"context_line":"                         \"\u0027ovf_metadata_properties\u0027 under \u0027image_format\u0027 \""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_cb01e3b0","line":200,"updated":"2015-08-31 06:23:45.000000000","message":"nit: if foo is None instead of \u0027\u003d\u003d\u0027","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a136c04fb32ec906ce7eea41dbd61eee2bdd91c5","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    def __init__(self):"},{"line_number":198,"context_line":"        self.interested_properties \u003d \\"},{"line_number":199,"context_line":"            CONF.image_format.ovf_metadata_properties"},{"line_number":200,"context_line":"        if self.interested_properties \u003d\u003d None:"},{"line_number":201,"context_line":"            LOG.warn(_LW(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":202,"context_line":"                         \u0027Glance conf file. Please set \u0027"},{"line_number":203,"context_line":"                         \"\u0027ovf_metadata_properties\u0027 under \u0027image_format\u0027 \""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_36d72825","line":200,"in_reply_to":"da20952f_cb01e3b0","updated":"2015-09-01 20:35:49.000000000","message":"Done","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e41bd18a56f87108d637e6f37735663d9aa4faae","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        self.interested_properties \u003d \\"},{"line_number":199,"context_line":"            CONF.image_format.ovf_metadata_properties"},{"line_number":200,"context_line":"        if self.interested_properties \u003d\u003d None:"},{"line_number":201,"context_line":"            LOG.warn(_LW(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":202,"context_line":"                         \u0027Glance conf file. Please set \u0027"},{"line_number":203,"context_line":"                         \"\u0027ovf_metadata_properties\u0027 under \u0027image_format\u0027 \""},{"line_number":204,"context_line":"                         \u0027group to a list of interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_8bd52b1d","line":201,"updated":"2015-08-31 06:23:45.000000000","message":"I don\u0027t understand what \u0027Interested OVF metadata\u0027 means?  But I know very little about OVF. Will users understand this?","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a136c04fb32ec906ce7eea41dbd61eee2bdd91c5","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        self.interested_properties \u003d \\"},{"line_number":199,"context_line":"            CONF.image_format.ovf_metadata_properties"},{"line_number":200,"context_line":"        if self.interested_properties \u003d\u003d None:"},{"line_number":201,"context_line":"            LOG.warn(_LW(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":202,"context_line":"                         \u0027Glance conf file. Please set \u0027"},{"line_number":203,"context_line":"                         \"\u0027ovf_metadata_properties\u0027 under \u0027image_format\u0027 \""},{"line_number":204,"context_line":"                         \u0027group to a list of interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_b6ca586d","line":201,"in_reply_to":"da20952f_8bd52b1d","updated":"2015-09-01 20:35:49.000000000","message":"Done","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":4992,"name":"malini-k-bhandaru","email":"malini.k.bhandaru@intel.com","username":"malini-k-bhandaru"},"change_message_id":"24798f5d8b55f2f9fa993f9e83761b653d92f70f","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        self.interested_properties \u003d \\"},{"line_number":199,"context_line":"            CONF.image_format.ovf_metadata_properties"},{"line_number":200,"context_line":"        if self.interested_properties \u003d\u003d None:"},{"line_number":201,"context_line":"            LOG.warn(_LW(\u0027Interested OVF metadata was not specified in \u0027"},{"line_number":202,"context_line":"                         \u0027Glance conf file. Please set \u0027"},{"line_number":203,"context_line":"                         \"\u0027ovf_metadata_properties\u0027 under \u0027image_format\u0027 \""},{"line_number":204,"context_line":"                         \u0027group to a list of interested properties\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_73014039","line":201,"in_reply_to":"da20952f_8bd52b1d","updated":"2015-09-01 15:39:47.000000000","message":"John, the space of OVF meta is large. Presently we are interested in perhaps 36. This is a config variable that the deployer can set.\nWould it make sense to rename this to OVF_METADATA_OF_INTEREST or OVF_METADATA_TO_EXTRACT","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":17561,"name":"Jakub Jasek","email":"jakub.jasek@tieto.com","username":"jjasek"},"change_message_id":"72f57505dc02dcd1bd810693e6464631a47a3fc3","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                properties \u003d self._parse_OVF(ovf)"},{"line_number":224,"context_line":"                ovf.close()"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"            for filename in filenames:"},{"line_number":227,"context_line":"                if any(filename.endswith(disk_format) for disk_format in"},{"line_number":228,"context_line":"                       CONF.image_format.disk_formats):"},{"line_number":229,"context_line":"                    disk_name \u003d filename"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_a8b1f148","line":226,"updated":"2015-08-31 08:21:54.000000000","message":"It is not detected if the .ova archive contains multiple dist images, that is not supported. (Only one of the disks is imported now)","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e41bd18a56f87108d637e6f37735663d9aa4faae","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                ovf.close()"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"            for filename in filenames:"},{"line_number":227,"context_line":"                if any(filename.endswith(disk_format) for disk_format in"},{"line_number":228,"context_line":"                       CONF.image_format.disk_formats):"},{"line_number":229,"context_line":"                    disk_name \u003d filename"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_6b29b736","line":227,"updated":"2015-08-31 06:23:45.000000000","message":"nit: Can do:\n\n if filename.endswith(tuple(CONF.image_format.disk_formats)):\n\n.endswith() accepts a tuple","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":12114,"name":"Kent Wang","email":"kent.wang@intel.com","username":"k.wang"},"change_message_id":"a136c04fb32ec906ce7eea41dbd61eee2bdd91c5","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                ovf.close()"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"            for filename in filenames:"},{"line_number":227,"context_line":"                if any(filename.endswith(disk_format) for disk_format in"},{"line_number":228,"context_line":"                       CONF.image_format.disk_formats):"},{"line_number":229,"context_line":"                    disk_name \u003d filename"},{"line_number":230,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_16030478","line":227,"in_reply_to":"da20952f_6b29b736","updated":"2015-09-01 20:35:49.000000000","message":"Done","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":17561,"name":"Jakub Jasek","email":"jakub.jasek@tieto.com","username":"jjasek"},"change_message_id":"72f57505dc02dcd1bd810693e6464631a47a3fc3","unresolved":false,"context_lines":[{"line_number":230,"context_line":""},{"line_number":231,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"},{"line_number":232,"context_line":"            if not disk:"},{"line_number":233,"context_line":"                LOG.error(_LE(\u0027Could not find valid disk image in OVA \u0027"},{"line_number":234,"context_line":"                              \u0027archive file\u0027))"},{"line_number":235,"context_line":"            return (disk, properties)"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_2884015c","line":233,"updated":"2015-08-31 08:21:54.000000000","message":"This if branch never entered, exception is raised at line 231 in this case when .ova archive does not contain .ovf file.","commit_id":"7efda7b392753061f7c788148034c609eb2e9ac3"},{"author":{"_account_id":4992,"name":"malini-k-bhandaru","email":"malini.k.bhandaru@intel.com","username":"malini-k-bhandaru"},"change_message_id":"48c6993e77ed5b550f53664943d4e0821ad49ace","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                 \"imported\") % {\"data_uri\": uri, \"task_id\": task_id})"},{"line_number":167,"context_line":"        data_iter \u003d script_utils.get_image_data_iter(uri)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        if image.container_format \u003d\u003d \u0027ova\u0027:"},{"line_number":170,"context_line":"            extractor \u003d OVAImageExtractor()"},{"line_number":171,"context_line":"            disk, properties \u003d extractor.extract(data_iter)"},{"line_number":172,"context_line":"            image.extra_properties.update(properties)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da20952f_e41f8c59","line":169,"updated":"2015-09-02 15:31:27.000000000","message":"please check for admin context/role as an interim security measure.","commit_id":"c5e72cdaaa97efdea7b5fc1d96fc20f208b94fb8"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"b0ab5f680c4085a2061dbdd21012436a21aaab67","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                 \"imported\") % {\"data_uri\": uri, \"task_id\": task_id})"},{"line_number":167,"context_line":"        data_iter \u003d script_utils.get_image_data_iter(uri)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        if image.container_format \u003d\u003d \u0027ova\u0027:"},{"line_number":170,"context_line":"            # NOTE: Right now OVA extract is limited to admin due to security"},{"line_number":171,"context_line":"            if image.context and image.context.is_admin:"},{"line_number":172,"context_line":"                extractor \u003d OVAImageExtractor()"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_6b6e8930","line":169,"updated":"2015-09-04 13:36:23.000000000","message":"I still think this should go in its own task and be executed before the import task. A couple of reasons below:\n\n1. The import step, more importantly the set_image_data step, should be responsible for getting the input data and store it as-is just like the API would do. The import task (or even better, the import flow) is now broken down into several sub-tasks that will be executed during the import process[0]. This code is literally hacking the `_ImportToStore` task, which relies on it to store the final, and already-processed, image data.\n\n2. As mentioned in the spec, the implementation requires space on disk. There are other tasks that have the same requirement (conversion) an they\u0027ve solved this issue by adding a `work_dir`[1] config option that should be configured in order for such task to be executed. Hacking this feature here does not guarantee there\u0027s space on disk and it does not use the dir configured by the operator. While the former won\u0027t be guaranteed even using the `work_dir` path, it\u0027ll surely confuse OPs if the path is configured and Glance extracts disks in other random path.\n\n3. Having this in a separate task will allow the implementation for separating concerns properly. The `OVFPRocess` task will extract the content, read the metadata, update the image and set the file_path to the extracted file in the `work_dir`\n\n4. In cases when conversion is requested, feeding the conversion task with the OVA file will simply endup in the image not being converted and the image metadata lost. Ideally, the `OVFProcess` task should be executed before the conversion, all the steps mentioned in #3 would happen and then the conversion task would be executed on the extracted disk image. Later in the flow, the conversion task would call `_ImportToStore` just like it does now.\n\n\n[0] https://github.com/openstack/glance/blob/master/glance/async/flows/base_import.py#L399\n[1] https://github.com/openstack/glance/blob/master/glance/common/config.py#L74","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"99c75d0ae833e5f5c6dea99825052fb02934f7da","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                 \"imported\") % {\"data_uri\": uri, \"task_id\": task_id})"},{"line_number":167,"context_line":"        data_iter \u003d script_utils.get_image_data_iter(uri)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        if image.container_format \u003d\u003d \u0027ova\u0027:"},{"line_number":170,"context_line":"            # NOTE: Right now OVA extract is limited to admin due to security"},{"line_number":171,"context_line":"            if image.context and image.context.is_admin:"},{"line_number":172,"context_line":"                extractor \u003d OVAImageExtractor()"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_83025908","line":169,"in_reply_to":"da20952f_6b6e8930","updated":"2015-09-09 07:56:43.000000000","message":"Agree with Flavio that this should be a separate task. Some optimizations if taking the tasks approach :-\n\n1. The OVA extraction can be done with the stream and does not need the work_dir (honestly, this comment should be on the spec, I will do that). \n\n2. If you don\u0027t extract the disk to a file, it may pose some trouble passing the stream to ImportToStore since it currently expects a URI (with file or http scheme) and not a file object. Just a heads up if doing (1).\n\nMay be we can write to a temp file for now and implement the optimizations later.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9ba65ba9db60d55df662af048cfe932bb40915b","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            if image.context and image.context.is_admin:"},{"line_number":172,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":173,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"},{"line_number":174,"context_line":"                image.extra_properties.update(properties)"},{"line_number":175,"context_line":"                if disk:"},{"line_number":176,"context_line":"                    data_iter \u003d disk"},{"line_number":177,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_57cfe87e","line":174,"updated":"2015-09-03 09:17:30.000000000","message":"This operates under the assumption that somewhere down the import path the `image.save` method will be called. Yay, domain model! \n\nI\u0027m not a fan of this because it modifies the image properties during the data storage steps. Other than setting the locations, I think we shouldn\u0027t be altering the image properties in this function. (Note that the locations are set in `image.set_data`.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"99c75d0ae833e5f5c6dea99825052fb02934f7da","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            if image.context and image.context.is_admin:"},{"line_number":172,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":173,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"},{"line_number":174,"context_line":"                image.extra_properties.update(properties)"},{"line_number":175,"context_line":"                if disk:"},{"line_number":176,"context_line":"                    data_iter \u003d disk"},{"line_number":177,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_e33f95b3","line":174,"in_reply_to":"da20952f_57cfe87e","updated":"2015-09-09 07:56:43.000000000","message":"@Flavio: If we take a tasks approach, we will have access to image_repo and can do image_repo.save after setting the properties like we do in introspection. \n\n@Kent: ^ So another reason to refactor this into its own task.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9ba65ba9db60d55df662af048cfe932bb40915b","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":173,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"},{"line_number":174,"context_line":"                image.extra_properties.update(properties)"},{"line_number":175,"context_line":"                if disk:"},{"line_number":176,"context_line":"                    data_iter \u003d disk"},{"line_number":177,"context_line":"            else:"},{"line_number":178,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_7778ec2b","line":175,"updated":"2015-09-03 09:17:30.000000000","message":"Why do we need this check? Is there a scenario where disk could be None?\n\nCould you elaborate?","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"2c3895c7fe4e57434e82488912c42c521a53372c","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":173,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"},{"line_number":174,"context_line":"                image.extra_properties.update(properties)"},{"line_number":175,"context_line":"                if disk:"},{"line_number":176,"context_line":"                    data_iter \u003d disk"},{"line_number":177,"context_line":"            else:"},{"line_number":178,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_cae2582f","line":175,"in_reply_to":"da20952f_7778ec2b","updated":"2015-09-03 16:48:32.000000000","message":"\u0027disk\u0027 can be None if there is no file in tar that ends with CONF.image_format.disk_formats (line 231).","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"c237f27d58117b0ee9642133dc338f4babc404c3","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                extractor \u003d OVAImageExtractor()"},{"line_number":173,"context_line":"                disk, properties \u003d extractor.extract(data_iter)"},{"line_number":174,"context_line":"                image.extra_properties.update(properties)"},{"line_number":175,"context_line":"                if disk:"},{"line_number":176,"context_line":"                    data_iter \u003d disk"},{"line_number":177,"context_line":"            else:"},{"line_number":178,"context_line":"                LOG.warn(_LW(\u0027OVA extract is limited to admin\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_4c0c09ec","line":175,"in_reply_to":"da20952f_cae2582f","updated":"2015-09-04 08:32:28.000000000","message":"Thanks In this case we have a problem in L231:\n\n\n    \u003e\u003e\u003e \"\".endswith(tuple(None))\n    Traceback (most recent call last):\n      File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n    TypeError: \u0027NoneType\u0027 object is not iterable\n    \u003e\u003e\u003e","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"99c75d0ae833e5f5c6dea99825052fb02934f7da","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        :returns: a tuple of extracted disk file object and dictionary of"},{"line_number":217,"context_line":"                  properties parsed from the OVF file"},{"line_number":218,"context_line":"        \"\"\""},{"line_number":219,"context_line":"        with tarfile.open(fileobj\u003dova) as tar_file:"},{"line_number":220,"context_line":"            disk_name, properties \u003d None, None"},{"line_number":221,"context_line":"            filenames \u003d tar_file.getnames()"},{"line_number":222,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_125a1951","line":219,"updated":"2015-09-09 07:56:43.000000000","message":"The fileobj could be non-seekable (when there is no conversion involved the fileobj is basically a http data stream). It\u0027s best to do :-\n\n with tarfile.open(mode\u003d\"r|\", fileobj\u003dova)\n\nCheck the tarfile doc for more explanation on working with streams.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"99c75d0ae833e5f5c6dea99825052fb02934f7da","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        \"\"\""},{"line_number":219,"context_line":"        with tarfile.open(fileobj\u003dova) as tar_file:"},{"line_number":220,"context_line":"            disk_name, properties \u003d None, None"},{"line_number":221,"context_line":"            filenames \u003d tar_file.getnames()"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"            ovf_filename \u003d next((filename for filename in filenames"},{"line_number":224,"context_line":"                                 if filename.endswith(\u0027.ovf\u0027)), None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_cdede0b8","line":221,"updated":"2015-09-09 07:56:43.000000000","message":"This assumes the fileobj is seekable. IMO, you may end up with streamerror later when you try to read the ovf. We need to process the tar file in a sequence. For some reference, you can look here https://github.com/openstack/nova/blame/master/nova/virt/vmwareapi/images.py#L421\n\nYou need to extract the metadata ovf and disk sequentially.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9ba65ba9db60d55df662af048cfe932bb40915b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                ovf.close()"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"            for filename in filenames:"},{"line_number":231,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"},{"line_number":232,"context_line":"                    disk_name \u003d filename"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_37a9147c","line":231,"updated":"2015-09-03 09:17:30.000000000","message":"What if it doesn\u0027t end with any of the `disk_formats` extensions but it\u0027s still a valid format? Is that possible? Should that scenario be considered? Do we have a better way to check this?","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"c237f27d58117b0ee9642133dc338f4babc404c3","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                ovf.close()"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"            for filename in filenames:"},{"line_number":231,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"},{"line_number":232,"context_line":"                    disk_name \u003d filename"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_8c2d4148","line":231,"in_reply_to":"da20952f_37a9147c","updated":"2015-09-04 08:32:28.000000000","message":"Just to be clear here. I think checking extensions is not a reliable way to verify the image format, I\u0027m not asking to support more formats than the ones supported by glance.\n\nFor example, I think it\u0027d be better to get the fileRef from the `DiskSection` in the OVF file and then get the filenames from the `File` element in the `References` section.\n\nLets use the info found in the OVF file, which is already parsed at this point, rather than figuring these things ourselves.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"99c75d0ae833e5f5c6dea99825052fb02934f7da","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                ovf.close()"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"            for filename in filenames:"},{"line_number":231,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"},{"line_number":232,"context_line":"                    disk_name \u003d filename"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_e8dbf27b","line":231,"in_reply_to":"da20952f_79f9837f","updated":"2015-09-09 07:56:43.000000000","message":"Agree with Flavio \u0026 Brian. The first disk in the DiskSection and it\u0027s corresponding FileRef\u0027s extension is one way to go. The other is to look for ovf:format tag in DiskSection which has a unique URI for each vendor format and that would be more reliable. Unfortunately, the OVF specification doesn\u0027t have enough information about the URI\u0027s themselves [1] but I found some Third party reference [2]\n\n\n[1] - https://www.dmtf.org/sites/default/files/standards/documents/DSP0243_2.1.0.pdf\n\n[2] - http://opennodecloud.com/howto/2013/12/25/howto-ON-ovf-reference.html","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f84610c893afdb1349430e38c444798fd7bac95e","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                ovf.close()"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"            for filename in filenames:"},{"line_number":231,"context_line":"                if filename.endswith(tuple(CONF.image_format.disk_formats)):"},{"line_number":232,"context_line":"                    disk_name \u003d filename"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"            disk \u003d tar_file.extractfile(disk_name)"}],"source_content_type":"text/x-python","patch_set":7,"id":"da20952f_79f9837f","line":231,"in_reply_to":"da20952f_8c2d4148","updated":"2015-09-04 14:20:01.000000000","message":"I agree entirely.  The task should take the OVF descriptor file to be authoritative in describing the payload of the package.","commit_id":"73adb7f59bc067bb760d1a93c89872505859ad25"}],"glance/tests/unit/async/flows/test_ovf_process.py":[{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"16150b748d7116fc839dc1ab9948f900b45bc14d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            self.assertRaises(AttributeError, iextractor.extract, ova_file)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    \"\"\" Nothing to test, not handled by code yet"},{"line_number":132,"context_line":"    def test_extract_ova_no_ovf(self):"},{"line_number":133,"context_line":"        testCONF.image_format.ovf_metadata_properties \u003d ["},{"line_number":134,"context_line":"            \u0027InstructionSetExtensionName\u0027,"},{"line_number":135,"context_line":"            \u0027InstructionName\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_2caeb985","line":132,"updated":"2015-09-15 14:43:13.000000000","message":"I think this is an important case to handle. It should fail with an error and the image, if already created, should be killed.","commit_id":"a0a49707e42cd7f3c9dce5448e25ad4624202b90"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class TestOvfProcessTask(test_utils.BaseTestCase):"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def setUp(self):"},{"line_number":35,"context_line":"        super(TestOvfProcessTask, self).setUp()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @mock.patch(\u0027glance.async.flows.ovf_process.CONF\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_66028db5","line":34,"updated":"2016-02-18 23:32:15.000000000","message":"I would prefer if this was setup the way we do other flow tests. It\u0027s just consistent and easy for maintainers. Plus, you don\u0027t have to mock the task_repo, image_repo in each test. \n\nReference:- https://github.com/openstack/glance/blob/master/glance/tests/unit/async/flows/test_introspect.py#L35","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class TestOvfProcessTask(test_utils.BaseTestCase):"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def setUp(self):"},{"line_number":35,"context_line":"        super(TestOvfProcessTask, self).setUp()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @mock.patch(\u0027glance.async.flows.ovf_process.CONF\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_f53a7797","line":34,"in_reply_to":"dae33548_66028db5","updated":"2016-02-22 06:53:44.000000000","message":"Done","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def setUp(self):"},{"line_number":35,"context_line":"        super(TestOvfProcessTask, self).setUp()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @mock.patch(\u0027glance.async.flows.ovf_process.CONF\u0027)"},{"line_number":38,"context_line":"    @mock.patch(\u0027os.rename\u0027)"},{"line_number":39,"context_line":"    def test_ovf_process_success(self, mock_rename, mock_conf):"},{"line_number":40,"context_line":"        config_file \u003d six.StringIO("}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_21922ea0","line":37,"updated":"2016-02-18 23:32:15.000000000","message":"We usually don\u0027t mock the CONF. You can override the config value like this :-\n\n self.config(key\u003d\u0027value\u0027,\n     group\u003d\"glance_store\")","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def setUp(self):"},{"line_number":35,"context_line":"        super(TestOvfProcessTask, self).setUp()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @mock.patch(\u0027glance.async.flows.ovf_process.CONF\u0027)"},{"line_number":38,"context_line":"    @mock.patch(\u0027os.rename\u0027)"},{"line_number":39,"context_line":"    def test_ovf_process_success(self, mock_rename, mock_conf):"},{"line_number":40,"context_line":"        config_file \u003d six.StringIO("}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_55424311","line":37,"in_reply_to":"dae33548_21922ea0","updated":"2016-02-22 06:53:44.000000000","message":"Done","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            \u0027\"ProcessorArchitecture\\\"]}\u0027)"},{"line_number":44,"context_line":"        mock_config \u003d mock.MagicMock()"},{"line_number":45,"context_line":"        mock_config.__enter__.return_value \u003d config_file"},{"line_number":46,"context_line":"        var_dir \u003d os.path.abspath(os.path.join(os.path.dirname(__file__),"},{"line_number":47,"context_line":"                                               \u0027../../../\u0027, \u0027var\u0027))"},{"line_number":48,"context_line":"        file_name \u003d os.path.join(var_dir, \u0027testserver.ova\u0027)"},{"line_number":49,"context_line":"        uri \u003d \u0027file://\u0027 + file_name"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_e6b21d05","line":46,"updated":"2016-02-18 23:32:15.000000000","message":"Nit: Since you use this in all tests,  you can move this to setup\n\n self.var_dir \u003d os.path.abspath ...","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            \u0027\"ProcessorArchitecture\\\"]}\u0027)"},{"line_number":44,"context_line":"        mock_config \u003d mock.MagicMock()"},{"line_number":45,"context_line":"        mock_config.__enter__.return_value \u003d config_file"},{"line_number":46,"context_line":"        var_dir \u003d os.path.abspath(os.path.join(os.path.dirname(__file__),"},{"line_number":47,"context_line":"                                               \u0027../../../\u0027, \u0027var\u0027))"},{"line_number":48,"context_line":"        file_name \u003d os.path.join(var_dir, \u0027testserver.ova\u0027)"},{"line_number":49,"context_line":"        uri \u003d \u0027file://\u0027 + file_name"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_3545ff16","line":46,"in_reply_to":"dae33548_e6b21d05","updated":"2016-02-22 06:53:44.000000000","message":"Done","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        mock_extracted \u003d mock.MagicMock()"},{"line_number":53,"context_line":"        mock_extracted.__enter__.return_value \u003d extracted_disk"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        mock_conf.find_file.return_value \u003d \"existing_file.conf\""},{"line_number":56,"context_line":"        mock_conf.image_format.disk_formats \u003d [\"vmdk\", \"qcow\"]"},{"line_number":57,"context_line":"        mock_conf.task.work_dir \u003d var_dir"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_01c9eaac","line":55,"updated":"2016-02-18 23:32:15.000000000","message":"You can also directly mock oslo_config.ConfigOpts.find_file but read my comment for L69, you may not need this for this test.","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        mock_extracted \u003d mock.MagicMock()"},{"line_number":53,"context_line":"        mock_extracted.__enter__.return_value \u003d extracted_disk"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        mock_conf.find_file.return_value \u003d \"existing_file.conf\""},{"line_number":56,"context_line":"        mock_conf.image_format.disk_formats \u003d [\"vmdk\", \"qcow\"]"},{"line_number":57,"context_line":"        mock_conf.task.work_dir \u003d var_dir"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_7547871d","line":55,"in_reply_to":"dae33548_01c9eaac","updated":"2016-02-22 06:53:44.000000000","message":"This is what I ended up doing (I noticed other places in OpenStack codebase doing the same). Also, see my reply to you L69 comment.","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        mock_extracted.__enter__.return_value \u003d extracted_disk"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        mock_conf.find_file.return_value \u003d \"existing_file.conf\""},{"line_number":56,"context_line":"        mock_conf.image_format.disk_formats \u003d [\"vmdk\", \"qcow\"]"},{"line_number":57,"context_line":"        mock_conf.task.work_dir \u003d var_dir"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        image \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_668f2d75","line":56,"updated":"2016-02-18 23:32:15.000000000","message":"As mentioned above you can use self.config to override conf values.","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        mock_extracted.__enter__.return_value \u003d extracted_disk"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        mock_conf.find_file.return_value \u003d \"existing_file.conf\""},{"line_number":56,"context_line":"        mock_conf.image_format.disk_formats \u003d [\"vmdk\", \"qcow\"]"},{"line_number":57,"context_line":"        mock_conf.task.work_dir \u003d var_dir"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        image \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_d55653d1","line":56,"in_reply_to":"dae33548_668f2d75","updated":"2016-02-22 06:53:44.000000000","message":"Done","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        processed_file \u003d open(file_name, \u0027rb\u0027)"},{"line_number":67,"context_line":"        mock_open \u003d mock.MagicMock()"},{"line_number":68,"context_line":"        mock_open.side_effect \u003d [mock_config, processed_file, mock_extracted]"},{"line_number":69,"context_line":"        with patch(\u0027six.moves.builtins.open\u0027, mock_open):"},{"line_number":70,"context_line":"            self.assertEqual(uri, oprocess.execute(\u0027test_image_id\u0027, uri))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        # Test disk content and expected property import"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_4152d335","line":69,"updated":"2016-02-18 23:32:15.000000000","message":"Mocking file.open and patching the same with file.open in L66 is not right just for the sake of mock_config to succeed. \n\nInstead, you can patch OVAExtractor.interested_properties directly with the properties to extract by mocking out _load_custom_properties (you can test load_custom_properties in a separate unit test). This will avoid mocking every file open. What do you think ?","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        processed_file \u003d open(file_name, \u0027rb\u0027)"},{"line_number":67,"context_line":"        mock_open \u003d mock.MagicMock()"},{"line_number":68,"context_line":"        mock_open.side_effect \u003d [mock_config, processed_file, mock_extracted]"},{"line_number":69,"context_line":"        with patch(\u0027six.moves.builtins.open\u0027, mock_open):"},{"line_number":70,"context_line":"            self.assertEqual(uri, oprocess.execute(\u0027test_image_id\u0027, uri))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        # Test disk content and expected property import"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_f561d7a9","line":69,"in_reply_to":"dae33548_4152d335","updated":"2016-02-22 06:53:44.000000000","message":"I realized that the tests were reading the sample ova packages from the physical disk anyways. So, no point trying to avoid the disk for the other files. So, I just move all the files read by the system-under-test to a temp dir in setup(). So, no need to mock open() anymore.","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            self.assertEqual(uri, oprocess.execute(\u0027test_image_id\u0027, uri))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        # Test disk content and expected property import"},{"line_number":73,"context_line":"        self.assertEqual(b\u0027ABCD\u0027, extracted_disk.getvalue())"},{"line_number":74,"context_line":"        image.extra_properties.update.assert_called_once_with("},{"line_number":75,"context_line":"            {\u0027cim_pasd_InstructionSetExtensionName\u0027: \u0027DMTF:x86:VT-d\u0027})"},{"line_number":76,"context_line":"        self.assertEqual(\u0027bare\u0027, image.container_format)"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_2147ce1e","line":73,"updated":"2016-02-18 23:32:15.000000000","message":"It took me while to figure out that b\u0027ABCD\u0027 is the last few bytes of the file. Can you instead check the file size of the extracted disk since you know the location ? Is that ok ?","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            self.assertEqual(uri, oprocess.execute(\u0027test_image_id\u0027, uri))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        # Test disk content and expected property import"},{"line_number":73,"context_line":"        self.assertEqual(b\u0027ABCD\u0027, extracted_disk.getvalue())"},{"line_number":74,"context_line":"        image.extra_properties.update.assert_called_once_with("},{"line_number":75,"context_line":"            {\u0027cim_pasd_InstructionSetExtensionName\u0027: \u0027DMTF:x86:VT-d\u0027})"},{"line_number":76,"context_line":"        self.assertEqual(\u0027bare\u0027, image.container_format)"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_b568ef88","line":73,"in_reply_to":"dae33548_2147ce1e","updated":"2016-02-22 06:53:44.000000000","message":"b\u0027ABCD\u0027 is the exact contents of the disk image file in the sample ova package used by the test. Because of the refactoring (see L69 comment) where I am using files on disk, I am forced to read them back for comparison.\n\nComparing only the size of the disk can be misleading. Hence I am comparing the exact contents.","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    @mock.patch(\u0027glance.async.flows.ovf_process.CONF\u0027)"},{"line_number":79,"context_line":"    @mock.patch(\u0027os.rename\u0027)"},{"line_number":80,"context_line":"    def test_ovf_process_no_config_file(self, mock_rename, mock_conf):"},{"line_number":81,"context_line":"        var_dir \u003d os.path.abspath(os.path.join(os.path.dirname(__file__),"},{"line_number":82,"context_line":"                                               \u0027../../../\u0027, \u0027var\u0027))"},{"line_number":83,"context_line":"        file_name \u003d os.path.join(var_dir, \u0027testserver.ova\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_e1e046fd","line":80,"updated":"2016-02-18 23:32:15.000000000","message":"Similar comments as above","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    @mock.patch(\u0027glance.async.flows.ovf_process.CONF\u0027)"},{"line_number":79,"context_line":"    @mock.patch(\u0027os.rename\u0027)"},{"line_number":80,"context_line":"    def test_ovf_process_no_config_file(self, mock_rename, mock_conf):"},{"line_number":81,"context_line":"        var_dir \u003d os.path.abspath(os.path.join(os.path.dirname(__file__),"},{"line_number":82,"context_line":"                                               \u0027../../../\u0027, \u0027var\u0027))"},{"line_number":83,"context_line":"        file_name \u003d os.path.join(var_dir, \u0027testserver.ova\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_f508b7db","line":80,"in_reply_to":"dae33548_e1e046fd","updated":"2016-02-22 06:53:44.000000000","message":"Done","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"36bd52d696a144e864fbaa7df318aeacceb319ef","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        with open(ova_file_name, \u0027rb\u0027) as ova_file:"},{"line_number":151,"context_line":"            self.assertRaises(RuntimeError, iextractor.extract, ova_file)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def test_parse_OVF_bad_ovf(self):"},{"line_number":154,"context_line":"        ovf_file_name \u003d os.path.abspath(os.path.join(os.path.dirname(__file__),"},{"line_number":155,"context_line":"                                                     \u0027../../../var\u0027,"},{"line_number":156,"context_line":"                                                     \u0027testserver-bad-ovf.ovf\u0027))"}],"source_content_type":"text/x-python","patch_set":24,"id":"dae33548_4b6672bb","line":153,"updated":"2016-02-18 23:32:15.000000000","message":"nit: lower_case preferred.","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":15225,"name":"Deepti Ramakrishna","email":"deepti.ramakrishna@intel.com","username":"dramakri"},"change_message_id":"a25a14b8c7634388032c1c4821a365b231290654","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        with open(ova_file_name, \u0027rb\u0027) as ova_file:"},{"line_number":151,"context_line":"            self.assertRaises(RuntimeError, iextractor.extract, ova_file)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def test_parse_OVF_bad_ovf(self):"},{"line_number":154,"context_line":"        ovf_file_name \u003d os.path.abspath(os.path.join(os.path.dirname(__file__),"},{"line_number":155,"context_line":"                                                     \u0027../../../var\u0027,"},{"line_number":156,"context_line":"                                                     \u0027testserver-bad-ovf.ovf\u0027))"}],"source_content_type":"text/x-python","patch_set":24,"id":"bae84128_55108301","line":153,"in_reply_to":"dae33548_4b6672bb","updated":"2016-02-22 06:53:44.000000000","message":"Done","commit_id":"7267a6dabca62eec0bc295a719b5010858f5130a"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"3128158c5ac24256feb0275e41c0d403aa668a86","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            \u0027      \"InstructionSetExtensionName\",\\n\u0027"},{"line_number":50,"context_line":"            \u0027      \"ProcessorArchitecture\"]\\n\u0027"},{"line_number":51,"context_line":"            \u0027}\\n\u0027)"},{"line_number":52,"context_line":"        self.config_file_name \u003d os.path.join(self.tempdir, \u0027ovf-metadata.json\u0027)"},{"line_number":53,"context_line":"        with open(self.config_file_name, \u0027w\u0027) as config_file:"},{"line_number":54,"context_line":"            config_file.write(interested_properties)"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"9aed3d3a_3e09a032","line":52,"updated":"2016-02-25 22:32:33.000000000","message":"Nit: rename to ovf_metadata_file so not to confuse with the glance config file.","commit_id":"b00048742e95541bbdb9ced4a30de464844cb4c0"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"3128158c5ac24256feb0275e41c0d403aa668a86","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        super(TestOvfProcessTask, self).tearDown()"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def _copy_ova_to_tmpdir(self, ova_name):"},{"line_number":70,"context_line":"        # Copies an ova pacakge to the tempdir for tempdir from where"},{"line_number":71,"context_line":"        # the system-under-test will read it from"},{"line_number":72,"context_line":"        shutil.copy(os.path.join(self.test_ova_dir, ova_name), self.tempdir)"},{"line_number":73,"context_line":"        return os.path.join(self.tempdir, ova_name)"}],"source_content_type":"text/x-python","patch_set":26,"id":"9aed3d3a_7ee208a1","line":70,"updated":"2016-02-25 22:32:33.000000000","message":"Nit: package.","commit_id":"b00048742e95541bbdb9ced4a30de464844cb4c0"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"3128158c5ac24256feb0275e41c0d403aa668a86","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        # testserver-disk1.vmdk contained in the testserver.ova package used"},{"line_number":92,"context_line":"        # by this test"},{"line_number":93,"context_line":"        self.assertEqual(b\u0027ABCD\u0027, content)"},{"line_number":94,"context_line":"        # \u0027DMTF:x86:VT-d\u0027 is the value in the testerver.ovf file in the"},{"line_number":95,"context_line":"        # testserver.ova package"},{"line_number":96,"context_line":"        self.image.extra_properties.update.assert_called_once_with("},{"line_number":97,"context_line":"            {\u0027cim_pasd_InstructionSetExtensionName\u0027: \u0027DMTF:x86:VT-d\u0027})"}],"source_content_type":"text/x-python","patch_set":26,"id":"9aed3d3a_e14cbd78","line":94,"updated":"2016-02-25 22:32:33.000000000","message":"Nit: testserver.ovf","commit_id":"b00048742e95541bbdb9ced4a30de464844cb4c0"}]}