)]}'
{"oslo_policy/_cache_handler.py":[{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"cdd7ca10d8177c6c4c863509e4237557e48766ce","unresolved":false,"context_lines":[{"line_number":45,"context_line":"            msg \u003d err.strerror"},{"line_number":46,"context_line":"            LOG.error(\u0027IO error loading %(filename)s: %(msg)s\u0027,"},{"line_number":47,"context_line":"                      {\u0027filename\u0027: filename, \u0027msg\u0027: msg})"},{"line_number":48,"context_line":"            return (False, \u0027\u0027)"},{"line_number":49,"context_line":"        cache_info[\u0027mtime\u0027] \u003d mtime"},{"line_number":50,"context_line":"        reloaded \u003d True"},{"line_number":51,"context_line":"    return (reloaded, cache_info[\u0027data\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_67e2daa2","line":48,"updated":"2019-07-12 18:56:23.000000000","message":"I think we still need to raise an exception here. Returning will make the calling code think we successfully loaded the file and it was empty.","commit_id":"74637b8fa6ab2a8244f38c4f61dcdfa042c5a008"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"0ba9ce2f9572f723ef2892896e3007ba4d20956d","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            with open(filename) as fap:"},{"line_number":46,"context_line":"                cache_info[\u0027data\u0027] \u003d fap.read()"},{"line_number":47,"context_line":"        except IOError as err:"},{"line_number":48,"context_line":"            msg \u003d err.strerror"},{"line_number":49,"context_line":"            err_code \u003d err.errno"},{"line_number":50,"context_line":"            LOG.error(\u0027IO error loading %(filename)s: %(msg)s\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_45968f95","line":47,"updated":"2019-07-16 15:04:19.000000000","message":"Maybe simply using \"PermissionError\" is enough in this case to avoid to check `EACCES` etc...\n\nhttps://docs.python.org/3.8/library/exceptions.html#PermissionError\n\n```\ntry:\n    with open(filename) as fap\n        cache_info[\u0027data\u0027] \u003d fap.read()\nexcept PermissionError as err:\n    Log.error(\"File permission bla bla\")\n    raise cfg.ConfigFilePermissionDeniedError((filename,))\n```","commit_id":"de855cb16c2b13eaa8200675ab735904b32430eb"},{"author":{"_account_id":27838,"name":"Vadym Markov","email":"vmarkov@mirantis.com","username":"vmarkov"},"change_message_id":"171dce358034b635a38b205089e6114699050b99","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            with open(filename) as fap:"},{"line_number":46,"context_line":"                cache_info[\u0027data\u0027] \u003d fap.read()"},{"line_number":47,"context_line":"        except IOError as err:"},{"line_number":48,"context_line":"            msg \u003d err.strerror"},{"line_number":49,"context_line":"            err_code \u003d err.errno"},{"line_number":50,"context_line":"            LOG.error(\u0027IO error loading %(filename)s: %(msg)s\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_a58c15d7","line":47,"in_reply_to":"7faddb67_45968f95","updated":"2019-07-17 11:19:56.000000000","message":"IOError is compatible with Python 2.7. It makes backporting of patch easier, if someone needs it","commit_id":"de855cb16c2b13eaa8200675ab735904b32430eb"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"ec4c38d2f59c421a7c4394cf754470f6ab06efed","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            with open(filename) as fap:"},{"line_number":46,"context_line":"                cache_info[\u0027data\u0027] \u003d fap.read()"},{"line_number":47,"context_line":"        except IOError as err:"},{"line_number":48,"context_line":"            msg \u003d err.strerror"},{"line_number":49,"context_line":"            err_code \u003d err.errno"},{"line_number":50,"context_line":"            LOG.error(\u0027IO error loading %(filename)s: %(msg)s\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_7b4ec0c1","line":47,"in_reply_to":"7faddb67_a58c15d7","updated":"2019-07-17 13:26:23.000000000","message":"Oh good point, you are right, I missed the py2.7 point of view.","commit_id":"de855cb16c2b13eaa8200675ab735904b32430eb"}],"oslo_policy/policy.py":[{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"7868e21c2d604db0f3eb893ff5c42b2bdc5a0866","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        try:"},{"line_number":791,"context_line":"            reloaded, data \u003d _cache_handler.read_cached_file("},{"line_number":792,"context_line":"                self._file_cache, path, force_reload\u003dforce_reload)"},{"line_number":793,"context_line":"        except cfg.ConfigFilesNotFoundError:"},{"line_number":794,"context_line":"            LOG.debug(\u0027Policy not found: %(path)s\u0027, {\u0027path\u0027: path})"},{"line_number":795,"context_line":"        except cfg.ConfigFilesPermissionDeniedError:"},{"line_number":796,"context_line":"            LOG.debug(\u0027Policy file permission error: %(path)s\u0027, {\u0027path\u0027: path})"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_59a42cba","line":793,"updated":"2019-07-17 14:35:46.000000000","message":"It\u0027s never raised, you need to manage this case too or leave it or let\u0027s added this case in a fix up\n\nSee my changes I6d3343319ca6519fd9d215f2e8d3fa38516e9f4d","commit_id":"a3a765c76f26b661971c670bb362f6d4a0a29099"},{"author":{"_account_id":27838,"name":"Vadym Markov","email":"vmarkov@mirantis.com","username":"vmarkov"},"change_message_id":"9d8fb0cc424b72882a58c74989d9e34d185d6357","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        try:"},{"line_number":791,"context_line":"            reloaded, data \u003d _cache_handler.read_cached_file("},{"line_number":792,"context_line":"                self._file_cache, path, force_reload\u003dforce_reload)"},{"line_number":793,"context_line":"        except cfg.ConfigFilesNotFoundError:"},{"line_number":794,"context_line":"            LOG.debug(\u0027Policy not found: %(path)s\u0027, {\u0027path\u0027: path})"},{"line_number":795,"context_line":"        except cfg.ConfigFilesPermissionDeniedError:"},{"line_number":796,"context_line":"            LOG.debug(\u0027Policy file permission error: %(path)s\u0027, {\u0027path\u0027: path})"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_8e0f9486","line":793,"in_reply_to":"7faddb67_59a42cba","updated":"2019-07-19 11:28:20.000000000","message":"Ok, let\u0027s merge your patch with OSError handling and rebase this one over it","commit_id":"a3a765c76f26b661971c670bb362f6d4a0a29099"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"d48333265a427d28f5fcfabec8e58b34d876cd3c","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        try:"},{"line_number":791,"context_line":"            reloaded, data \u003d _cache_handler.read_cached_file("},{"line_number":792,"context_line":"                self._file_cache, path, force_reload\u003dforce_reload)"},{"line_number":793,"context_line":"        except cfg.ConfigFilesNotFoundError:"},{"line_number":794,"context_line":"            LOG.debug(\u0027Policy not found: %(path)s\u0027, {\u0027path\u0027: path})"},{"line_number":795,"context_line":"        except cfg.ConfigFilesPermissionDeniedError:"},{"line_number":796,"context_line":"            LOG.debug(\u0027Policy file permission error: %(path)s\u0027, {\u0027path\u0027: path})"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_b667464d","line":793,"in_reply_to":"7faddb67_8e0f9486","updated":"2019-07-19 13:57:51.000000000","message":"We can\u0027t proceed like this... my patch is mostly introduced to test your changes, so my changes depends on your changes. \n\n2 possible solutions to move forward:\n1. you remove the except `cfg.ConfigFilesNotFoundError` part in your (https://review.opendev.org/#/c/670571/4/oslo_policy/policy.py) and we reintroduce it on my changes, and you keep focus on the permission error.\n2. you grab my my OS.Error part on https://review.opendev.org/#/c/671309/3/oslo_policy/_cache_handler.py (grab all my changes on this file) and my patch become only focused on the test part.\n\nI personaly prefer the 1st solution to keep focus on the permission part and keep the semantic of your changes in the git history.","commit_id":"a3a765c76f26b661971c670bb362f6d4a0a29099"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"6431e45581323d528be27752096704125324ebc1","unresolved":false,"context_lines":[{"line_number":791,"context_line":"            reloaded, data \u003d _cache_handler.read_cached_file("},{"line_number":792,"context_line":"                self._file_cache, path, force_reload\u003dforce_reload)"},{"line_number":793,"context_line":"        except cfg.ConfigFilesNotFoundError:"},{"line_number":794,"context_line":"            LOG.debug(\u0027Policy not found: %(path)s\u0027, {\u0027path\u0027: path})"},{"line_number":795,"context_line":"        except cfg.ConfigFilesPermissionDeniedError:"},{"line_number":796,"context_line":"            LOG.debug(\u0027Policy file permission error: %(path)s\u0027, {\u0027path\u0027: path})"},{"line_number":797,"context_line":"        if reloaded or not self.rules:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_3fe569dc","line":794,"updated":"2019-07-19 16:14:15.000000000","message":"I don\u0027t think we should have both error (in the other file) and debug logs. That\u0027s redundant and will just clutter up the logs in debug mode.","commit_id":"a3a765c76f26b661971c670bb362f6d4a0a29099"}]}