)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"6eeb2636f084b80e29d6ebc293b7b2949171e08e","unresolved":false,"context_lines":[{"line_number":25,"context_line":"If image path do not starts with any of the \u0027mountpoint\u0027 provided"},{"line_number":26,"context_line":"in metadata JSON file then error is logged and empty"},{"line_number":27,"context_line":"dictionary is returned to glance api."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: I76b0a0de54e909fb0ec90bd7f9972da76a2501eb"},{"line_number":30,"context_line":"Closes-Bug: 1401778"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"1a930d6b_9f8ff5cb","line":28,"updated":"2015-01-23 14:08:29.000000000","message":"This should be tagged with DocImpact, maybe also APIImpact, I\u0027m not sure. Any thoughts?","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"ce5d8f951ef7db008a4a6a2d09a7b216b6331c59","unresolved":false,"context_lines":[{"line_number":25,"context_line":"If image path do not starts with any of the \u0027mountpoint\u0027 provided"},{"line_number":26,"context_line":"in metadata JSON file then error is logged and empty"},{"line_number":27,"context_line":"dictionary is returned to glance api."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: I76b0a0de54e909fb0ec90bd7f9972da76a2501eb"},{"line_number":30,"context_line":"Closes-Bug: 1401778"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"1a930d6b_2578afc3","line":28,"in_reply_to":"1a930d6b_9f8ff5cb","updated":"2015-01-23 16:23:34.000000000","message":"+1 I agree","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"}],"glance_store/_drivers/filesystem.py":[{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"f9876b205a79a0aad23dc769bcc035f775289e7d","unresolved":false,"context_lines":[{"line_number":406,"context_line":"                jsonschema.validate(metadata, SCHEMA_FOR_DICT)"},{"line_number":407,"context_line":"            # If metadata is of type list (list of dicts)"},{"line_number":408,"context_line":"            # i.e. - it contains multiple mountpoints"},{"line_number":409,"context_line":"            elif isinstance(metadata, list):"},{"line_number":410,"context_line":"                jsonschema.validate(metadata, SCHEMA_FOR_LIST_OF_DICTS)"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"            glance_store.check_location_metadata(metadata)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_563b0838","line":409,"updated":"2014-12-16 15:14:14.000000000","message":"why not put it this way: \n\nif isinstance(metadata, dict):\n    jsonschema.validate(metadata, SCHEMA_FOR_DICT)\nelse:\n    jsonschema.validate(metadata, SCHEMA_FOR_LIST_OF_DICTS)\n\nAt least you won\u0027t get the illusion that some other option is possible and this option is not processed.","commit_id":"81905ddaa8cd0f0daa5aa41cb4c6ec0987033072"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"f9876b205a79a0aad23dc769bcc035f775289e7d","unresolved":false,"context_lines":[{"line_number":411,"context_line":""},{"line_number":412,"context_line":"            glance_store.check_location_metadata(metadata)"},{"line_number":413,"context_line":"            # Get metadata dict containing \u0027id\u0027 and \u0027mountpoint\u0027"},{"line_number":414,"context_line":"            metadata \u003d self._get_metadata_from_path(filepath, metadata)"},{"line_number":415,"context_line":"            return metadata"},{"line_number":416,"context_line":"        except jsonschema.exceptions.ValidationError as jev:"},{"line_number":417,"context_line":"            reason \u003d _LE(\u0027The JSON in the metadata file %(file)s is not valid \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_964ad0aa","line":414,"updated":"2014-12-16 15:14:14.000000000","message":"I\u0027d combine these 2 lines in one:\n\nreturn self._get_metadata_from_path(filepath, metadata)","commit_id":"81905ddaa8cd0f0daa5aa41cb4c6ec0987033072"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"f9876b205a79a0aad23dc769bcc035f775289e7d","unresolved":false,"context_lines":[{"line_number":413,"context_line":"            # Get metadata dict containing \u0027id\u0027 and \u0027mountpoint\u0027"},{"line_number":414,"context_line":"            metadata \u003d self._get_metadata_from_path(filepath, metadata)"},{"line_number":415,"context_line":"            return metadata"},{"line_number":416,"context_line":"        except jsonschema.exceptions.ValidationError as jev:"},{"line_number":417,"context_line":"            reason \u003d _LE(\u0027The JSON in the metadata file %(file)s is not valid \u0027"},{"line_number":418,"context_line":"                         \u0027and it can not be used: %(jev)s  \\nAn empty \u0027"},{"line_number":419,"context_line":"                         \u0027dictionary will be returned to the client.\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_76a9e423","line":416,"updated":"2014-12-16 15:14:14.000000000","message":"I see no point in having a separate try\\catch block for each exception (ValidationError, BackendException and ValueError). The error message is pretty much the same and no processing (except \"return {}\") is done.","commit_id":"81905ddaa8cd0f0daa5aa41cb4c6ec0987033072"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"909ff6cddba45eacf0b5d7a14a6b98cdc5ae6728","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                    \"mountpoint defined in metadata : %(metadata)s\")"},{"line_number":379,"context_line":"                  % dict(path\u003dpath, metadata\u003dmetadata))"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        if isinstance(metadata, dict):"},{"line_number":382,"context_line":"            if not path.startswith(metadata[\u0027mountpoint\u0027]):"},{"line_number":383,"context_line":"                raise exceptions.BackendException(reason)"},{"line_number":384,"context_line":"            return metadata"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_6465dc43","line":381,"updated":"2014-12-24 10:53:22.000000000","message":"can we do this by a single loop logic at #387 sample directly? what in my mind like this:\n\n    metadata \u003d list(metadata)  # list([]) \u003d\u003d [] \n    for instance_meta in metadata:\n        # ...\n\nThen we don\u0027t need to check by isinstance()","commit_id":"4feec0dd0a453860fe2609008b7e3c6b013c2871"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"909ff6cddba45eacf0b5d7a14a6b98cdc5ae6728","unresolved":false,"context_lines":[{"line_number":400,"context_line":"            with open(metadata_file, \u0027r\u0027) as fptr:"},{"line_number":401,"context_line":"                metadata \u003d jsonutils.load(fptr)"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"            if isinstance(metadata, dict):"},{"line_number":404,"context_line":"                # If metadata is of type dictionary"},{"line_number":405,"context_line":"                # i.e. - it contains only one mountpoint"},{"line_number":406,"context_line":"                jsonschema.validate(metadata, SCHEMA_FOR_DICT)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_c403f03f","line":403,"updated":"2014-12-24 10:53:22.000000000","message":"I have a same idea on this like above one, and then I think we only need one SCHEMA_FOR_LIST_OF_DICTS, due to we just care about 1) if metadata contains keys \u0027id\u0027, \u0027mountpoint\u0027 and 2) all the values are string only. Actually we don\u0027t really care if it\u0027s list or a single metadata.","commit_id":"4feec0dd0a453860fe2609008b7e3c6b013c2871"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"909ff6cddba45eacf0b5d7a14a6b98cdc5ae6728","unresolved":false,"context_lines":[{"line_number":417,"context_line":"            reason \u003d _LE(\u0027The JSON in the metadata file %(file)s is not valid \u0027"},{"line_number":418,"context_line":"                         \u0027and it can not be used: %(vee)s.\\nAn empty \u0027"},{"line_number":419,"context_line":"                         \u0027dictionary will be returned to the client.\u0027"},{"line_number":420,"context_line":"                         % dict(file\u003dmetadata_file, vee\u003dunicode(vee)))"},{"line_number":421,"context_line":"            LOG.error(reason)"},{"line_number":422,"context_line":"            return {}"},{"line_number":423,"context_line":"        except IOError as ioe:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_040178ae","line":420,"updated":"2014-12-24 10:53:22.000000000","message":"normally we use glance_store.common.utils.exception_to_str() instead of unicode to string exception object.","commit_id":"4feec0dd0a453860fe2609008b7e3c6b013c2871"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"c08e4c6061cf84409ffce08396ca74c04b1b8a6a","unresolved":false,"context_lines":[{"line_number":372,"context_line":"        for instance_meta in metadata:"},{"line_number":373,"context_line":"            if path.startswith(instance_meta[\u0027mountpoint\u0027]):"},{"line_number":374,"context_line":"                return instance_meta"},{"line_number":375,"context_line":"        else:"},{"line_number":376,"context_line":"            raise exceptions.BackendException(reason)"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"    def _get_metadata(self, filepath):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_710af979","line":375,"updated":"2014-12-24 12:52:06.000000000","message":"Else is redundant (besides, indent is wrong)","commit_id":"3f77c2179dfa102c6a23683073f705d4191d1cf2"},{"author":{"_account_id":8300,"name":"Abhijeet Malawade","email":"abhijeet.malawade@gmail.com","username":"abhijeet"},"change_message_id":"c2eeab2075f4b009df81b21f03ab576d6c1cb12e","unresolved":false,"context_lines":[{"line_number":372,"context_line":"        for instance_meta in metadata:"},{"line_number":373,"context_line":"            if path.startswith(instance_meta[\u0027mountpoint\u0027]):"},{"line_number":374,"context_line":"                return instance_meta"},{"line_number":375,"context_line":"        else:"},{"line_number":376,"context_line":"            raise exceptions.BackendException(reason)"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"    def _get_metadata(self, filepath):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_54bb2bc3","line":375,"in_reply_to":"3a961159_710af979","updated":"2014-12-24 13:19:27.000000000","message":"Hi Inessa,\n\nThis else block is of \u0027for\u0027 loop. (it is not of \u0027if\u0027 condition)\n\nIf image path do not start with any of the \u0027mointpoint\u0027 given in metadata then it will go in to this else block and raise exception.","commit_id":"3f77c2179dfa102c6a23683073f705d4191d1cf2"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"e9e13dff9748cf20ea50c31184395ba4cdf7a98a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                      \"associated with this store.  The file must \""},{"line_number":58,"context_line":"                      \"contain a valid JSON dict. Also this dict \""},{"line_number":59,"context_line":"                      \"should contain keys \u0027id\u0027, \u0027mountpoint\u0027 and all the \""},{"line_number":60,"context_line":"                      \"values should be string only.\")),"},{"line_number":61,"context_line":"    cfg.IntOpt(\u0027filesystem_store_file_perm\u0027,"},{"line_number":62,"context_line":"               default\u003d0,"},{"line_number":63,"context_line":"               help\u003d_(\"The required permission for created image file. \""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_682ad52d","line":60,"updated":"2014-12-29 22:51:52.000000000","message":"We should probably rewrite this to not call them dicts. They\u0027re not called that in JSON. They\u0027re simply objects. We should refrain from saying \"Also this ...\". Something like\n\n    The file must contain a valid JSON object. The object should contain the keys \"id\" and \"mountpoint\". The value for both keys should be \"string\".","commit_id":"051db68ef618a3569fd1295fabfbf517c0131bfb"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"e9e13dff9748cf20ea50c31184395ba4cdf7a98a","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                      \"default permission of the file. This value will be \""},{"line_number":69,"context_line":"                      \"decoded as an octal digit.\"))]"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"SCHEMA_FOR_LIST_OF_DICTS \u003d {"},{"line_number":72,"context_line":"    \"type\": \"array\","},{"line_number":73,"context_line":"    \"items\": {"},{"line_number":74,"context_line":"        \"type\": \"object\","}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_c8b74939","line":71,"updated":"2014-12-29 22:51:52.000000000","message":"This needs a better name. What about MULTI_FILESYSTEM_METADATA_SCHEMA ?","commit_id":"051db68ef618a3569fd1295fabfbf517c0131bfb"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"e9e13dff9748cf20ea50c31184395ba4cdf7a98a","unresolved":false,"context_lines":[{"line_number":369,"context_line":"        :param metadata: Metadata as list of dicts"},{"line_number":370,"context_line":"        \"\"\""},{"line_number":371,"context_line":"        reason \u003d (_(\"The image path %(path)s does not match with any of the \""},{"line_number":372,"context_line":"                    \"mountpoint defined in metadata : %(metadata)s\")"},{"line_number":373,"context_line":"                  % dict(path\u003dpath, metadata\u003dmetadata))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        for instance_meta in metadata:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_a81bdd1a","line":372,"updated":"2014-12-29 22:51:52.000000000","message":"Remove the space before the : please","commit_id":"051db68ef618a3569fd1295fabfbf517c0131bfb"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"e9e13dff9748cf20ea50c31184395ba4cdf7a98a","unresolved":false,"context_lines":[{"line_number":375,"context_line":"        for instance_meta in metadata:"},{"line_number":376,"context_line":"            if path.startswith(instance_meta[\u0027mountpoint\u0027]):"},{"line_number":377,"context_line":"                return instance_meta"},{"line_number":378,"context_line":"        else:"},{"line_number":379,"context_line":"            raise exceptions.BackendException(reason)"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"    def _get_metadata(self, filepath):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_c8182921","line":378,"updated":"2014-12-29 22:51:52.000000000","message":"This does not need to be an else here.","commit_id":"051db68ef618a3569fd1295fabfbf517c0131bfb"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"77eb552addfed491a5b594f9974933ae2933de53","unresolved":false,"context_lines":[{"line_number":322,"context_line":"                    store_name\u003d\"filesystem\", reason\u003dreason)"},{"line_number":323,"context_line":"            except IOError as ioe:"},{"line_number":324,"context_line":"                reason \u003d _(\u0027The path for the metadata file %(file)s could \u0027"},{"line_number":325,"context_line":"                           \u0027not be opened: \u0027"},{"line_number":326,"context_line":"                           \u0027%(ioe)s.\u0027) % dict(file\u003dmetadata_file,"},{"line_number":327,"context_line":"                                              ioe\u003dutils.exception_to_str(ioe))"},{"line_number":328,"context_line":"                LOG.error(reason)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_ef5a6050","line":325,"updated":"2015-01-12 03:51:53.000000000","message":"nit: s/opened/accessed/ ?","commit_id":"8fafe11454bf503bf33cd757ec0818a8413fbc61"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"77eb552addfed491a5b594f9974933ae2933de53","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        filesize \u003d os.path.getsize(filepath)"},{"line_number":389,"context_line":"        return filepath, filesize"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    def _get_metadata_from_path(self, path, metadata):"},{"line_number":392,"context_line":"        \"\"\""},{"line_number":393,"context_line":"        Return metadata dictionary containing \u0027id\u0027 and \u0027mountpoint\u0027 if"},{"line_number":394,"context_line":"        metadata is provided as list of dictionaries."}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_2a8d367d","line":391,"updated":"2015-01-12 03:51:53.000000000","message":"I think we\u0027d better to merge this two functions and do a necessary refactoring?","commit_id":"8fafe11454bf503bf33cd757ec0818a8413fbc61"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"b6816800ea670d305673cc0e438a8f0abe5964aa","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        \"\"\""},{"line_number":410,"context_line":"        metadata_file \u003d self.conf.glance_store.filesystem_store_metadata_file"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"        if self.FILESYSTEM_STORE_METADATA:"},{"line_number":413,"context_line":"            try:"},{"line_number":414,"context_line":"                for instance_meta in self.FILESYSTEM_STORE_METADATA:"},{"line_number":415,"context_line":"                    if filepath.startswith(instance_meta[\u0027mountpoint\u0027]):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_58807549","line":412,"updated":"2015-01-13 01:52:27.000000000","message":"At this point we have a few paths through the code:\n\nWe can return {} immediately.\nWe can return instance_meta (if we find an appropriate metadata instance in the filesystem store).\nWe don\u0027t find something in the store, or we an exception and return {}.\n\nI think we can make the code more readable if we do the following:\n\n    if not self.FILESYSTEM_STORE_METADATA:\n        return {}\n\n    # de-indent everything currently nested\n    return {}\n\nYou\u0027re adding 1 extra line and improving readability greatly. I think it\u0027s a significant improvement.","commit_id":"3eb280b89c5aeb30d8b4f175de3cc9acb8ffba5a"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"e692f89e495ade037cd091e80f60fd601aad6bff","unresolved":false,"context_lines":[{"line_number":422,"context_line":"                                 metadata\u003dself.FILESYSTEM_STORE_METADATA))"},{"line_number":423,"context_line":"                LOG.error(reason)"},{"line_number":424,"context_line":"                raise exceptions.BackendException(reason)"},{"line_number":425,"context_line":"            except (exceptions.BackendException) as vee:"},{"line_number":426,"context_line":"                reason \u003d (_LE(\u0027The JSON in the metadata file %(file)s \u0027"},{"line_number":427,"context_line":"                              \u0027is not valid and it can not be used: %(vee)s.\u0027"},{"line_number":428,"context_line":"                              \u0027\\nAn empty dictionary will be \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_162952f2","line":425,"updated":"2015-01-13 12:00:47.000000000","message":"Raising an exception (exceptions.BackendException) that will be caught in the next line is imho not a very good idea.\n\nI\u0027d suggest something like the following (considering Ian\u0027s remarks):\n\nif not self.FILESYSTEM_STORE_METADATA:\n    return {}\nif filepath.startswith(instance_meta[\u0027mountpoint\u0027]):\n    return instance_meta\nreason \u003d (_(\"The image path ... \") )\nLOG.error(reason)\n# by the way, why do you log the error twice?\nreason \u003d (_LE(\"The JSON in .... \")) \nLOG.error(reason)\nreturn {}","commit_id":"3eb280b89c5aeb30d8b4f175de3cc9acb8ffba5a"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"23e1f1b4b706d5b3d16a4890e51cf193d0b39ea2","unresolved":false,"context_lines":[{"line_number":422,"context_line":"                                 metadata\u003dself.FILESYSTEM_STORE_METADATA))"},{"line_number":423,"context_line":"                LOG.error(reason)"},{"line_number":424,"context_line":"                raise exceptions.BackendException(reason)"},{"line_number":425,"context_line":"            except (exceptions.BackendException) as vee:"},{"line_number":426,"context_line":"                reason \u003d (_LE(\u0027The JSON in the metadata file %(file)s \u0027"},{"line_number":427,"context_line":"                              \u0027is not valid and it can not be used: %(vee)s.\u0027"},{"line_number":428,"context_line":"                              \u0027\\nAn empty dictionary will be \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_ca739d4c","line":425,"in_reply_to":"3a961159_162952f2","updated":"2015-01-13 13:23:54.000000000","message":"As for me I would rewrite it to be more pythonic:\n\nif self.FILESYSTEM_STORE_METADATA:\n    for instance_meta in self.FILESYSTEM_STORE_METADATA:\n        if filepath.startswith(instance_meta[\u0027mountpoint\u0027]):\n            return instance_meta\n    else:\n        # else for for-loop shows that the loop has been finished normally\n        reason \u003d (_LE(\"The image path %(path)s does not match with \" ...\n        LOG.error(reason)\nelse:\n    # remove else-block for outer if if you want to combine the error messages\n    reason \u003d (_LE(\u0027The JSON in the metadata file %(file)s \u0027...\n    LOG.error(reason)\nreturn {}","commit_id":"3eb280b89c5aeb30d8b4f175de3cc9acb8ffba5a"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"06a2ff83bb5ca04c3ca87c3241d44180cd0d221d","unresolved":false,"context_lines":[{"line_number":422,"context_line":"                                 metadata\u003dself.FILESYSTEM_STORE_METADATA))"},{"line_number":423,"context_line":"                LOG.error(reason)"},{"line_number":424,"context_line":"                raise exceptions.BackendException(reason)"},{"line_number":425,"context_line":"            except (exceptions.BackendException) as vee:"},{"line_number":426,"context_line":"                reason \u003d (_LE(\u0027The JSON in the metadata file %(file)s \u0027"},{"line_number":427,"context_line":"                              \u0027is not valid and it can not be used: %(vee)s.\u0027"},{"line_number":428,"context_line":"                              \u0027\\nAn empty dictionary will be \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_4f116a52","line":425,"in_reply_to":"3a961159_162952f2","updated":"2015-01-13 21:54:32.000000000","message":"Good catch Inessa.","commit_id":"3eb280b89c5aeb30d8b4f175de3cc9acb8ffba5a"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"06a2ff83bb5ca04c3ca87c3241d44180cd0d221d","unresolved":false,"context_lines":[{"line_number":422,"context_line":"                                 metadata\u003dself.FILESYSTEM_STORE_METADATA))"},{"line_number":423,"context_line":"                LOG.error(reason)"},{"line_number":424,"context_line":"                raise exceptions.BackendException(reason)"},{"line_number":425,"context_line":"            except (exceptions.BackendException) as vee:"},{"line_number":426,"context_line":"                reason \u003d (_LE(\u0027The JSON in the metadata file %(file)s \u0027"},{"line_number":427,"context_line":"                              \u0027is not valid and it can not be used: %(vee)s.\u0027"},{"line_number":428,"context_line":"                              \u0027\\nAn empty dictionary will be \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_bd04c718","line":425,"in_reply_to":"3a961159_ca739d4c","updated":"2015-01-13 21:54:32.000000000","message":"Mike, using for/else here is not more pythonic. If you never return from the if-block inside the for-loop you can be confident in logging the error regardless. The else would be \"more pythonic\" if there were a break and we were trying to find the instance_meta to use later on.","commit_id":"3eb280b89c5aeb30d8b4f175de3cc9acb8ffba5a"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"e692f89e495ade037cd091e80f60fd601aad6bff","unresolved":false,"context_lines":[{"line_number":430,"context_line":"                          % dict(file\u003dmetadata_file,"},{"line_number":431,"context_line":"                                 vee\u003dutils.exception_to_str(vee)))"},{"line_number":432,"context_line":"                LOG.error(reason)"},{"line_number":433,"context_line":"            except Exception as ex:"},{"line_number":434,"context_line":"                LOG.exception(_LE(\u0027An error occurred while processing the \u0027"},{"line_number":435,"context_line":"                                  \u0027storage systems meta data file: %s. \u0027"},{"line_number":436,"context_line":"                                  \u0027An empty dictionary will be \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_5c24db5f","line":433,"updated":"2015-01-13 12:00:47.000000000","message":"Seems that this except block will never be reached, as no func call in _get_metadata() can lead to an Exception","commit_id":"3eb280b89c5aeb30d8b4f175de3cc9acb8ffba5a"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"dc5d612709717fed13812cfbf4df575bd33f1da2","unresolved":false,"context_lines":[{"line_number":410,"context_line":"        if not self.FILESYSTEM_STORE_METADATA:"},{"line_number":411,"context_line":"            return {}"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        for instance_meta in self.FILESYSTEM_STORE_METADATA:"},{"line_number":414,"context_line":"            if filepath.startswith(instance_meta[\u0027mountpoint\u0027]):"},{"line_number":415,"context_line":"                return instance_meta"},{"line_number":416,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_ef5f76e2","line":413,"updated":"2015-01-13 21:54:42.000000000","message":"Since you\u0027re not performing gymnastics with exceptions now, I think undoing my previous suggestion will be acceptable if you\u0027d rather revert that particular part, e.g., \n\n    if self.FILESYSTEM_STORE_METADATA:\n        for instance_meta in self.FILESYSTEM_STORE_METADATA:\n            if filepath.startswith(instance_meta[\u0027mountpoint\u0027]):\n                return instance_meta","commit_id":"9a4b2646f4be6b4c49f716dbce0ab958f31b71d9"},{"author":{"_account_id":10300,"name":"Pranali Deore","email":"pranalideore@gmail.com","username":"PranaliDeore"},"change_message_id":"8baa2e037985f7609f88396d2b47f6764a2d400d","unresolved":false,"context_lines":[{"line_number":410,"context_line":"        if not self.FILESYSTEM_STORE_METADATA:"},{"line_number":411,"context_line":"            return {}"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        for instance_meta in self.FILESYSTEM_STORE_METADATA:"},{"line_number":414,"context_line":"            if filepath.startswith(instance_meta[\u0027mountpoint\u0027]):"},{"line_number":415,"context_line":"                return instance_meta"},{"line_number":416,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_29db2f7e","line":413,"in_reply_to":"3a961159_ef5f76e2","updated":"2015-01-14 07:24:08.000000000","message":"Done","commit_id":"9a4b2646f4be6b4c49f716dbce0ab958f31b71d9"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"d41551c9a1efbcf8b80b44be024e8b894c2a4140","unresolved":false,"context_lines":[{"line_number":291,"context_line":""},{"line_number":292,"context_line":"        self._create_image_directories(directory_paths)"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        # Validating metadata against json schema at configuration stage and"},{"line_number":295,"context_line":"        # caching metadata and using it directly when new image being added."},{"line_number":296,"context_line":"        if self.conf.glance_store.filesystem_store_metadata_file:"},{"line_number":297,"context_line":"            metadata_file \u003d \\"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_fd3a7396","line":294,"updated":"2015-01-15 23:16:00.000000000","message":"Could this be moved to its own function? configure_add() is already a fairly long function, and the validation of metadata can be cleanly separated out.","commit_id":"4284fc0152ae43c5ca1ca6c943d5d4d8b9e4f6ec"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"562527faf592a5c0c466e2557709982ec8be21b9","unresolved":false,"context_lines":[{"line_number":291,"context_line":""},{"line_number":292,"context_line":"        self._create_image_directories(directory_paths)"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        # Validating metadata against json schema at configuration stage and"},{"line_number":295,"context_line":"        # caching metadata and using it directly when new image being added."},{"line_number":296,"context_line":"        if self.conf.glance_store.filesystem_store_metadata_file:"},{"line_number":297,"context_line":"            metadata_file \u003d \\"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_8546237d","line":294,"in_reply_to":"3a961159_fd3a7396","updated":"2015-01-19 22:50:41.000000000","message":"I agree with Louis. This is probably best as a method like _validate_metadata","commit_id":"4284fc0152ae43c5ca1ca6c943d5d4d8b9e4f6ec"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"2aa0ead7c265cc1e39301f0b906e8f2b61c7962c","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                    raise exceptions.BadStoreConfiguration("},{"line_number":234,"context_line":"                        store_name\u003d\"filesystem\", reason\u003dreason)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def _validate_metadata(self):"},{"line_number":237,"context_line":"        \"\"\""},{"line_number":238,"context_line":"        Validating metadata against json schema."},{"line_number":239,"context_line":"        If metadata is valid then caching metadata and using it while"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_b79b3472","line":236,"updated":"2015-01-20 17:18:43.000000000","message":"Can metadata_file be passed as a parameter, instead?","commit_id":"acf34c4d315a0fde9fb3a9148249e675a61567cb"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"2aa0ead7c265cc1e39301f0b906e8f2b61c7962c","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        :raise BadStoreConfiguration exception if metadata is not valid."},{"line_number":243,"context_line":"        \"\"\""},{"line_number":244,"context_line":"        if self.conf.glance_store.filesystem_store_metadata_file:"},{"line_number":245,"context_line":"            metadata_file \u003d \\"},{"line_number":246,"context_line":"                self.conf.glance_store.filesystem_store_metadata_file"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_3796643f","line":244,"updated":"2015-01-20 17:18:43.000000000","message":"And move this if statement into the consuming function.","commit_id":"acf34c4d315a0fde9fb3a9148249e675a61567cb"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"d365bbbd84e42ba0ce8310aec50bd17f1616434e","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        :raise BadStoreConfiguration exception if metadata is not valid."},{"line_number":243,"context_line":"        \"\"\""},{"line_number":244,"context_line":"        if self.conf.glance_store.filesystem_store_metadata_file:"},{"line_number":245,"context_line":"            metadata_file \u003d \\"},{"line_number":246,"context_line":"                self.conf.glance_store.filesystem_store_metadata_file"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_9c1f3fb8","line":245,"updated":"2015-01-20 17:07:12.000000000","message":"Would this assignment fit if it were moved outside of the if-block? For example,\n\n    metadata_file \u003d self.conf.glance_store.filesystem_store_metadata_file\n    if metadata_file:\n        # ...\n\nIf not, we can work around this with something like:\n\n    store_conf \u003d self.conf.glance_store\n    if store_conf.filesystem_store_metadata_file:\n         metadata_file \u003d store_conf.filesystem_store_metadata_file","commit_id":"acf34c4d315a0fde9fb3a9148249e675a61567cb"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"2aa0ead7c265cc1e39301f0b906e8f2b61c7962c","unresolved":false,"context_lines":[{"line_number":581,"context_line":"                self._delete_partial(filepath, image_id)"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"        checksum_hex \u003d checksum.hexdigest()"},{"line_number":584,"context_line":"        metadata \u003d self._get_metadata(filepath)"},{"line_number":585,"context_line":""},{"line_number":586,"context_line":"        LOG.debug(_(\"Wrote %(bytes_written)d bytes to %(filepath)s with \""},{"line_number":587,"context_line":"                    \"checksum %(checksum_hex)s\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_77f46c88","line":584,"updated":"2015-01-20 17:18:43.000000000","message":"Then change this to:\n\n    if self.conf.glance_store.filesystem_store_metadata_file:\n        self._get_metadata(self.conf.glance_store.filesystem_store_metadata_file)\n\nIan\u0027s comment could still apply here.","commit_id":"acf34c4d315a0fde9fb3a9148249e675a61567cb"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"cb5064c74e315ff8229f6fd0469f292eab98d1a1","unresolved":false,"context_lines":[{"line_number":581,"context_line":"                self._delete_partial(filepath, image_id)"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"        checksum_hex \u003d checksum.hexdigest()"},{"line_number":584,"context_line":"        metadata \u003d self._get_metadata(filepath)"},{"line_number":585,"context_line":""},{"line_number":586,"context_line":"        LOG.debug(_(\"Wrote %(bytes_written)d bytes to %(filepath)s with \""},{"line_number":587,"context_line":"                    \"checksum %(checksum_hex)s\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_b5350948","line":584,"in_reply_to":"1a930d6b_9fd58c45","updated":"2015-01-21 15:04:25.000000000","message":"Sorry, not sure how I ended up commenting on this line! I meant to do so on line 337 instead. You got the right idea, though. Thanks!","commit_id":"acf34c4d315a0fde9fb3a9148249e675a61567cb"},{"author":{"_account_id":8300,"name":"Abhijeet Malawade","email":"abhijeet.malawade@gmail.com","username":"abhijeet"},"change_message_id":"9d2f724df7e539f45e6d3a2a211ff7b731909d89","unresolved":false,"context_lines":[{"line_number":581,"context_line":"                self._delete_partial(filepath, image_id)"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"        checksum_hex \u003d checksum.hexdigest()"},{"line_number":584,"context_line":"        metadata \u003d self._get_metadata(filepath)"},{"line_number":585,"context_line":""},{"line_number":586,"context_line":"        LOG.debug(_(\"Wrote %(bytes_written)d bytes to %(filepath)s with \""},{"line_number":587,"context_line":"                    \"checksum %(checksum_hex)s\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_9fd58c45","line":584,"in_reply_to":"3a961159_77f46c88","updated":"2015-01-21 14:02:12.000000000","message":"Hi Louis,\n\nHere \u0027filepath\u0027 is not metadata JSON file path, It is path of image file (i.e.- where image is stored on filesysytem).","commit_id":"acf34c4d315a0fde9fb3a9148249e675a61567cb"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"e5a815b87165c30779ccaddcb52a0bfc24f40714","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            \"id\": {\"type\": \"string\"},"},{"line_number":77,"context_line":"            \"mountpoint\": {\"type\": \"string\"}"},{"line_number":78,"context_line":"        },"},{"line_number":79,"context_line":"        \"required\": [\"id\", \"mountpoint\"],"},{"line_number":80,"context_line":"    }"},{"line_number":81,"context_line":"}"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_dcd7cb21","line":79,"updated":"2015-01-23 13:29:19.000000000","message":"The implementation looks good to me, thanks for you follow up my comments. However I have some thoughts like to share with you.\n\nI think this \u0027require\u0027 on location metadata comes from download plug on nova side (correct me if I\u0027m wrong please). As original idea, this approach is designed around NFS shared storage which connected between glance-api node(s) and compute nodes, which was used to speed up vm provisioning (to prepare root disk efficiently), so from glance\u0027s perspective, I\u0027m a _little_ worried if this kind of limitation is too heavy, I see you\u0027re fixing a bug, but I also see the metadata of image location probably should be unlimited, e.g. do we need to suppose filesystem store are always based on NFS? what should we do if other download plugin is added to nova side (by community or cloud provider customized) which uses an optimized approach for filesystem store and needed particular metadata as well? In sort, do we like to make a hard global limitation on the metadata of filesystem stored image just for a particular plugin on Nova side?\n\nI don\u0027t want to be a blocker, but I\u0027d like to know your idea on this point. Thanks.","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":8300,"name":"Abhijeet Malawade","email":"abhijeet.malawade@gmail.com","username":"abhijeet"},"change_message_id":"687bf6a2fd2b9f043886c37eaf5572c1dbe34274","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            \"id\": {\"type\": \"string\"},"},{"line_number":77,"context_line":"            \"mountpoint\": {\"type\": \"string\"}"},{"line_number":78,"context_line":"        },"},{"line_number":79,"context_line":"        \"required\": [\"id\", \"mountpoint\"],"},{"line_number":80,"context_line":"    }"},{"line_number":81,"context_line":"}"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_ae07c580","line":79,"in_reply_to":"1a930d6b_3891b252","updated":"2015-01-27 09:22:11.000000000","message":"Hi,\n\nInstead of checking \u0027id\u0027 and \u0027mountpoint\u0027 in \u0027configure_add\u0027 method directly, can we add a Validator class for this specific case and use whenever needed. \n\nFor example : \n\n* Add following parameters to glance-api.conf\n\nfilesystem_store_metadata_schema_file\u003d/etc/glance/filesystem_store_metadata_schema.json\nfilesystem_store_metadata_schema_driver\u003dglance.store.filesystem.NoveFileSystemSchemaValidator\n\nAdd following class to implement schema validation code.\nglance_store._drivers.filesytem.py\n\nAdd following new classes\n1. Base class\nFileSystemSchemaValidator\n   def validate_metadata(metadata_file)\n   def get_metadata(filepath)\n   \n2. Subclass\nNovaFileSystemSchemaValidator\n    def validate_metadata(metadata_file)\n    def get_metadata(filepath)\n       # validate id and mountpoint.\n\nWith this approach, it is possible to add more FileSystemSchemaValidator for validating metadata contents as per schema.\n\nPlease let me know if you have any better solution for this.","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"ce5d8f951ef7db008a4a6a2d09a7b216b6331c59","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            \"id\": {\"type\": \"string\"},"},{"line_number":77,"context_line":"            \"mountpoint\": {\"type\": \"string\"}"},{"line_number":78,"context_line":"        },"},{"line_number":79,"context_line":"        \"required\": [\"id\", \"mountpoint\"],"},{"line_number":80,"context_line":"    }"},{"line_number":81,"context_line":"}"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_3891b252","line":79,"in_reply_to":"1a930d6b_9f4a95aa","updated":"2015-01-23 16:23:34.000000000","message":"So I think there\u0027s some confusion. Using \"required\" in JSON Schema does not exclude additional properties. In other words, a consumer can send extra properties in this, this just requires that id and mountpoint are at least there.\n\nThat said, I agree with the concern that it might be best if mountpoint isn\u0027t a required property.","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"6eeb2636f084b80e29d6ebc293b7b2949171e08e","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            \"id\": {\"type\": \"string\"},"},{"line_number":77,"context_line":"            \"mountpoint\": {\"type\": \"string\"}"},{"line_number":78,"context_line":"        },"},{"line_number":79,"context_line":"        \"required\": [\"id\", \"mountpoint\"],"},{"line_number":80,"context_line":"    }"},{"line_number":81,"context_line":"}"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_9f4a95aa","line":79,"in_reply_to":"1a930d6b_dcd7cb21","updated":"2015-01-23 14:08:29.000000000","message":"I do agree with Zhi Yan here, we should not enforce for single specific use case. Personally I think this needs quite a bit documentation for those special deployments rather than schema enforcement for everybody.","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"f294f3d03a13c9ef2043e4c5917860523f7b0bbe","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        \"\"\""},{"line_number":238,"context_line":"        Validating metadata against json schema."},{"line_number":239,"context_line":"        If metadata is valid then caching metadata and using it while"},{"line_number":240,"context_line":"        creating new image."},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        :param metadata_file: JSON metadata file path"},{"line_number":243,"context_line":"        :raise BadStoreConfiguration exception if metadata is not valid."}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_f55dd149","line":240,"updated":"2015-01-21 15:16:19.000000000","message":"Nit: clean up phrasing here\n\n    \"\"\"\n    Validate metadata against json schema.\n    If metadata is valid then cache metadata and use it when\n    creating new image.","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"6eeb2636f084b80e29d6ebc293b7b2949171e08e","unresolved":false,"context_lines":[{"line_number":237,"context_line":"        \"\"\""},{"line_number":238,"context_line":"        Validating metadata against json schema."},{"line_number":239,"context_line":"        If metadata is valid then caching metadata and using it while"},{"line_number":240,"context_line":"        creating new image."},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        :param metadata_file: JSON metadata file path"},{"line_number":243,"context_line":"        :raise BadStoreConfiguration exception if metadata is not valid."}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_ff839901","line":240,"in_reply_to":"1a930d6b_f55dd149","updated":"2015-01-23 14:08:29.000000000","message":"NIT: Docstring should not start with empty line, so please:\n\"\"\" Validate metadata against json schema.\n\n+ Louis concerns if possible\n\nhttp://docs.openstack.org/developer/hacking/#docstrings","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"f294f3d03a13c9ef2043e4c5917860523f7b0bbe","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        metadata_file \u003d self.conf.glance_store.filesystem_store_metadata_file"},{"line_number":336,"context_line":"        if metadata_file:"},{"line_number":337,"context_line":"            self._validate_metadata(metadata_file)"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"    def _check_directory_paths(self, datadir_path, directory_paths):"},{"line_number":340,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_153c1d2f","line":337,"updated":"2015-01-21 15:16:19.000000000","message":"+1 this is easier to read","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"6eeb2636f084b80e29d6ebc293b7b2949171e08e","unresolved":false,"context_lines":[{"line_number":396,"context_line":"        return filepath, filesize"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    def _get_metadata(self, filepath):"},{"line_number":399,"context_line":"        \"\"\""},{"line_number":400,"context_line":"        Return metadata dictionary containing \u0027id\u0027 and \u0027mountpoint\u0027 if"},{"line_number":401,"context_line":"        metadata is provided as list of dictionaries."},{"line_number":402,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_3f7d211d","line":399,"updated":"2015-01-23 14:08:29.000000000","message":"NIT: Docstring should not start with empty line.","commit_id":"3d95f30508f1bdf9a858ca70163b8a4bf3d5717c"}],"tests/unit/test_filesystem_store.py":[{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"f9876b205a79a0aad23dc769bcc035f775289e7d","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        self.assertEqual(metadata, in_metadata)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def test_add_check_metadata_bad_data(self):"},{"line_number":193,"context_line":"        in_metadata \u003d {\u0027akey\u0027: 10}  # only unicode is allowed"},{"line_number":194,"context_line":"        location, size, checksum, metadata \u003d self._store_image(in_metadata)"},{"line_number":195,"context_line":"        self.assertEqual(metadata, {})"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_d609b8de","line":193,"updated":"2014-12-16 15:14:14.000000000","message":"Here test semantics is wrong: the test fails because of absence of \u0027id\u0027 and \u0027mountpoint\u0027 (due to \"Failed validating \u0027required\u0027 in schema\" error) and not due to \"only unicode is allowed\".","commit_id":"81905ddaa8cd0f0daa5aa41cb4c6ec0987033072"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"c08e4c6061cf84409ffce08396ca74c04b1b8a6a","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def test_add_check_metadata_with_invalid_mountpoint_location(self):"},{"line_number":217,"context_line":"        # Image is stored at \u0027/tmp/\u0027 location, but if we give invalid"},{"line_number":218,"context_line":"        # mountpoint location (i.e.- if image path do not starts with"},{"line_number":219,"context_line":"        # mountpoint) in json file then empty dictionary is returned as"},{"line_number":220,"context_line":"        # image metadata."},{"line_number":221,"context_line":"        in_metadata \u003d {\u0027id\u0027: \u0027abcdefg\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_71cf5929","line":218,"updated":"2014-12-24 12:52:06.000000000","message":"I\u0027d move the part about empty dictionary being returned to docstring","commit_id":"3f77c2179dfa102c6a23683073f705d4191d1cf2"}]}