)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4262d909dd3a8601954ede39d03fac59d1cc4b06","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Use oslo.config instead of argparse."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Changing arg consumption from argparse to oslo.config in"},{"line_number":10,"context_line":"order to also provide behavior control using config files."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iec4dab763b973b70c98077cb29708acd9cbbcec4"},{"line_number":13,"context_line":"Signed-off-by: Moisés Guimarães de Medeiros \u003cmoguimar@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_aecd0f5d","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":58},"updated":"2018-12-19 17:32:49.000000000","message":"Is there a link to the discussion where this was raised? I\u0027m struggling to see how/why you\u0027d want to use a config file for this stuff.","commit_id":"fcdbaac0cd6ca9ed2cc6e1854c03a0ef43c8e458"}],"oslo_policy/shell.py":[{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"8b56d7e4b30c01eee6a94808c7aaa8e3d1063c3c","unresolved":false,"context_lines":[{"line_number":113,"context_line":"    conf()"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    policy \u003d open(conf.policy, \"rb\", 0)"},{"line_number":116,"context_line":"    access \u003d open(conf.access, \"rb\", 0)"},{"line_number":117,"context_line":"    target \u003d open(conf.target, \"rb\", 0) if conf.target else None"},{"line_number":118,"context_line":"    is_admin \u003d conf.is_admin.lower() \u003d\u003d \"true\""},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_2b414d0a","line":116,"updated":"2018-12-14 11:37:59.000000000","message":"I would reorganize the code and use the \"with\" statement to auto-close the files.\n\nIt will also be more natural, in my opinion, for the tool function to take file names instead of the open files, they can be opened inside it.","commit_id":"7de2304347e2381e3138aaa075381755a9feafc9"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"8d8c9fbd4b7a869552ce73118106d67d086d7ae5","unresolved":false,"context_lines":[{"line_number":113,"context_line":"    conf()"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    policy \u003d open(conf.policy, \"rb\", 0)"},{"line_number":116,"context_line":"    access \u003d open(conf.access, \"rb\", 0)"},{"line_number":117,"context_line":"    target \u003d open(conf.target, \"rb\", 0) if conf.target else None"},{"line_number":118,"context_line":"    is_admin \u003d conf.is_admin.lower() \u003d\u003d \"true\""},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_8827eb1d","line":116,"in_reply_to":"3f79a3b5_0be571cf","updated":"2018-12-19 16:57:06.000000000","message":"The \"with\" statement can be useful to sanitize code execution but in the previous version we use type\u003dargparse.Filetype so we obtain a file object (cf. https://docs.python.org/3/library/argparse.html#type and https://docs.python.org/3/library/functions.html#open).\n\nThis code have a similar behavior than when we use argparse. It\u0027s due to the file descriptor created below. \n\nArgparse doesn\u0027t close the created ile descriptor (cf. https://github.com/python/cpython/blob/master/Lib/argparse.py#L1198 )\n\nSo I personaly prefer to changes the \"tool\" function to take string filename instead of file objects and lets this function manage the file life cycle I think is more proper.\n\nAn another useful change will be to add \"FileOpt\" or something like that to oslo.config to implicitly manage the same use cases and deal directly with file descriptor.\n\nThoughts?","commit_id":"7de2304347e2381e3138aaa075381755a9feafc9"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"a87cf1ad8a0659c35f40c429e568c53347d76d59","unresolved":false,"context_lines":[{"line_number":113,"context_line":"    conf()"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    policy \u003d open(conf.policy, \"rb\", 0)"},{"line_number":116,"context_line":"    access \u003d open(conf.access, \"rb\", 0)"},{"line_number":117,"context_line":"    target \u003d open(conf.target, \"rb\", 0) if conf.target else None"},{"line_number":118,"context_line":"    is_admin \u003d conf.is_admin.lower() \u003d\u003d \"true\""},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_4b512932","line":116,"in_reply_to":"3f79a3b5_0be571cf","updated":"2018-12-14 12:09:56.000000000","message":"ok :)\nsee my comments in the other patch","commit_id":"7de2304347e2381e3138aaa075381755a9feafc9"},{"author":{"_account_id":27954,"name":"Moisés Guimarães de Medeiros","email":"guimaraes@pm.me","username":"moguimar"},"change_message_id":"78d9ddf54a027768a3d9b02b74181dc8ff9b923d","unresolved":false,"context_lines":[{"line_number":113,"context_line":"    conf()"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    policy \u003d open(conf.policy, \"rb\", 0)"},{"line_number":116,"context_line":"    access \u003d open(conf.access, \"rb\", 0)"},{"line_number":117,"context_line":"    target \u003d open(conf.target, \"rb\", 0) if conf.target else None"},{"line_number":118,"context_line":"    is_admin \u003d conf.is_admin.lower() \u003d\u003d \"true\""},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_0be571cf","line":116,"in_reply_to":"3f79a3b5_2b414d0a","updated":"2018-12-14 11:53:30.000000000","message":"Hi Grzegorz,\n\nI totally agree with your solution, my intent with this patch was to switch from argparse to oslo.config without refactoring on the rest of the elements, does this makes sense? I can either do the full refactoring here on in a following commit.","commit_id":"7de2304347e2381e3138aaa075381755a9feafc9"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"216f2774c9af5e91ecac8c3a2274ea6fd82f2c76","unresolved":false,"context_lines":[{"line_number":113,"context_line":"    conf()"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    policy \u003d open(conf.policy, \"rb\", 0)"},{"line_number":116,"context_line":"    access \u003d open(conf.access, \"rb\", 0)"},{"line_number":117,"context_line":"    target \u003d open(conf.target, \"rb\", 0) if conf.target else None"},{"line_number":118,"context_line":"    is_admin \u003d conf.is_admin.lower() \u003d\u003d \"true\""},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_f18be61f","line":116,"in_reply_to":"3f79a3b5_8827eb1d","updated":"2018-12-19 18:19:21.000000000","message":"See the following patch here: https://review.openstack.org/616659\n\nMoises implemented it with a \"with\" statement.","commit_id":"7de2304347e2381e3138aaa075381755a9feafc9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4262d909dd3a8601954ede39d03fac59d1cc4b06","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        \u0027rule\u0027,"},{"line_number":109,"context_line":"        help\u003d\u0027rule to test\u0027))"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    conf.register_cli_opt(cfg.StrOpt("},{"line_number":112,"context_line":"        \u0027is_admin\u0027,"},{"line_number":113,"context_line":"        help\u003d\u0027set is_admin\u003dTrue on the credentials used for the evaluation\u0027,"},{"line_number":114,"context_line":"        default\u003d\"\"))"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    conf()"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_2eda5f1a","line":114,"range":{"start_line":111,"start_character":0,"end_line":114,"end_character":20},"updated":"2018-12-19 17:32:49.000000000","message":"Shouldn\u0027t this be converted to a BoolOpt now? I thought that was one of the reasons for doing this?","commit_id":"fcdbaac0cd6ca9ed2cc6e1854c03a0ef43c8e458"}]}
