)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"4b3f696ac59a6db40afcdb71b885238479d66305","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix None dereference in _get_api_version"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In some situations vault will not provide KV API version"},{"line_number":10,"context_line":"in the options structure."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Fall back to assume version \u00272\u0027 in these situations."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_601449eb","line":10,"updated":"2018-11-01 21:12:25.000000000","message":"This doesn\u0027t seem quite right. The exception \"\u0027NoneType\u0027 object is not subscriptable\" suggests that either the data or options structure is None, not that version is missing from them. If that\u0027s an expected situation then this logic is probably fine, but it doesn\u0027t sound that way.","commit_id":"f78b3b1e25be8e3c1ad691d1f333825dced4999f"},{"author":{"_account_id":27954,"name":"Moisés Guimarães de Medeiros","email":"guimaraes@pm.me","username":"moguimar"},"change_message_id":"f03a6818c4fcb29e3023e19b9cf004dfbda31a3d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix None dereference in _get_api_version"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In some situations vault will not provide KV API version"},{"line_number":10,"context_line":"in the options structure."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Fall back to assume version \u00272\u0027 in these situations."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_d1b26a31","line":10,"in_reply_to":"3f79a3b5_601449eb","updated":"2018-11-07 08:13:59.000000000","message":"I would also like to know in which scenario it returns None instead of the version number itself.","commit_id":"f78b3b1e25be8e3c1ad691d1f333825dced4999f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a114d22f5d0e5f94ab7f527f8c1c81205d7f2f79","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In some situations vault will not provide KV API version"},{"line_number":10,"context_line":"in the options structure. Vault documentation [1] doesn\u0027t cover cases"},{"line_number":11,"context_line":"when KV API version is not provided. After some tests I found that it"},{"line_number":12,"context_line":"depends on how Vault is deployed: we can specify KV API version via CLI"},{"line_number":13,"context_line":"argument or via \u0027option\u0027 argunment in JSON config."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Fall back to assume version \u00272\u0027 in these situations."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf51134e_caf2e506","line":13,"range":{"start_line":11,"start_character":37,"end_line":13,"end_character":50},"updated":"2020-06-15 14:46:11.000000000","message":"I wonder if it\u0027s possible to request clarification on this from Hashicorp? We\u0027re hardly the first people to hit this?","commit_id":"427682081e0f722cdda6a2da1ad5db802ee2e8f1"}],"castellan/key_manager/vault_key_manager.py":[{"author":{"_account_id":27954,"name":"Moisés Guimarães de Medeiros","email":"guimaraes@pm.me","username":"moguimar"},"change_message_id":"f03a6818c4fcb29e3023e19b9cf004dfbda31a3d","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            try:"},{"line_number":115,"context_line":"                self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027][\u0027version\u0027]"},{"line_number":116,"context_line":"            except (KeyError, TypeError):"},{"line_number":117,"context_line":"                self._vault_kv_version \u003d \u00272\u0027"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        return self._vault_kv_version"},{"line_number":120,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_11d4624a","line":117,"range":{"start_line":117,"start_character":41,"end_line":117,"end_character":44},"updated":"2018-11-07 08:13:59.000000000","message":"Any specific reason why we should fallback to version 2 instead of version 1?","commit_id":"f78b3b1e25be8e3c1ad691d1f333825dced4999f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bacfac8ccfc105ff46c26a70134b4d3e188a216f","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            try:"},{"line_number":115,"context_line":"                self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027][\u0027version\u0027]"},{"line_number":116,"context_line":"            except (KeyError, TypeError):"},{"line_number":117,"context_line":"                self._vault_kv_version \u003d \u00272\u0027"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        return self._vault_kv_version"},{"line_number":120,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_97cafd6c","line":117,"range":{"start_line":117,"start_character":41,"end_line":117,"end_character":44},"in_reply_to":"3f79a3b5_11d4624a","updated":"2019-12-18 14:55:07.000000000","message":"Yeah, this reads like the API response we\u0027re receiving back is not what we expected. Why would we even default to a version instead of failing because we don\u0027t understand this?","commit_id":"f78b3b1e25be8e3c1ad691d1f333825dced4999f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a114d22f5d0e5f94ab7f527f8c1c81205d7f2f79","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        else:"},{"line_number":120,"context_line":"            _resp \u003d resp.json()"},{"line_number":121,"context_line":"            try:"},{"line_number":122,"context_line":"                self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027][\u0027version\u0027]"},{"line_number":123,"context_line":"            except (KeyError, TypeError):"},{"line_number":124,"context_line":"                # NOTE(e0ne): fallback to v2 since it\u0027s used by default now"},{"line_number":125,"context_line":"                # if vesrion discovery API doesn\u0027t work."}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_4a91b599","line":122,"range":{"start_line":122,"start_character":16,"end_line":122,"end_character":76},"updated":"2020-06-15 14:46:11.000000000","message":"Could we do\n\n  self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027].get(\u0027version\u0027, \u00272\u0027)\n\ninstead? I\u0027m worried about getting garbage back and interpreting this as a special condition. Assuming only the \u0027version\u0027 field is missing, this will establish that things look some bit sane, anyway.","commit_id":"427682081e0f722cdda6a2da1ad5db802ee2e8f1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2c0565637c5c53b887ff64a4b3deb7cf3dfb0ba1","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        else:"},{"line_number":120,"context_line":"            _resp \u003d resp.json()"},{"line_number":121,"context_line":"            try:"},{"line_number":122,"context_line":"                self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027][\u0027version\u0027]"},{"line_number":123,"context_line":"            except (KeyError, TypeError):"},{"line_number":124,"context_line":"                # NOTE(e0ne): fallback to v2 since it\u0027s used by default now"},{"line_number":125,"context_line":"                # if vesrion discovery API doesn\u0027t work."}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_f2dccf19","line":122,"range":{"start_line":122,"start_character":16,"end_line":122,"end_character":76},"in_reply_to":"bf51134e_44ab641c","updated":"2020-06-17 15:31:37.000000000","message":"\u003e I like that. I\u0027m also inclined to think that we should default to 2\n \u003e and handle this by config other than trying to guess.\n\nMaking this configurable sounds reasonable, yes.","commit_id":"427682081e0f722cdda6a2da1ad5db802ee2e8f1"},{"author":{"_account_id":27954,"name":"Moisés Guimarães de Medeiros","email":"guimaraes@pm.me","username":"moguimar"},"change_message_id":"8f60573fe0295f18f7c07686e82e6d9847d929cb","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        else:"},{"line_number":120,"context_line":"            _resp \u003d resp.json()"},{"line_number":121,"context_line":"            try:"},{"line_number":122,"context_line":"                self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027][\u0027version\u0027]"},{"line_number":123,"context_line":"            except (KeyError, TypeError):"},{"line_number":124,"context_line":"                # NOTE(e0ne): fallback to v2 since it\u0027s used by default now"},{"line_number":125,"context_line":"                # if vesrion discovery API doesn\u0027t work."}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_44ab641c","line":122,"range":{"start_line":122,"start_character":16,"end_line":122,"end_character":76},"in_reply_to":"bf51134e_4a91b599","updated":"2020-06-15 20:18:19.000000000","message":"I like that. I\u0027m also inclined to think that we should default to 2 and handle this by config other than trying to guess.","commit_id":"427682081e0f722cdda6a2da1ad5db802ee2e8f1"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"a6e4f1dac54150802074d7ee01fa38556d63388f","unresolved":false,"context_lines":[{"line_number":120,"context_line":"            _resp \u003d resp.json()"},{"line_number":121,"context_line":"            try:"},{"line_number":122,"context_line":"                self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027][\u0027version\u0027]"},{"line_number":123,"context_line":"            except (KeyError, TypeError):"},{"line_number":124,"context_line":"                # NOTE(e0ne): fallback to v2 since it\u0027s used by default now"},{"line_number":125,"context_line":"                # if vesrion discovery API doesn\u0027t work."},{"line_number":126,"context_line":"                self._vault_kv_version \u003d \u00272\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_7c2418c7","line":123,"range":{"start_line":123,"start_character":30,"end_line":123,"end_character":39},"updated":"2020-06-17 14:41:41.000000000","message":"I agree with Stephen\u0027s comment, also in which case do you think facing `TypeError`?\n\nAs much I think the KeyError is possible in this case and could be avoided by the Stephen\u0027s approach, as much I don\u0027t seen when a TypeError could be raised here [1]. \n\nMaybe I\u0027m wrong.\n\n[1] https://docs.python.org/3/library/exceptions.html#TypeError","commit_id":"427682081e0f722cdda6a2da1ad5db802ee2e8f1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2c0565637c5c53b887ff64a4b3deb7cf3dfb0ba1","unresolved":false,"context_lines":[{"line_number":120,"context_line":"            _resp \u003d resp.json()"},{"line_number":121,"context_line":"            try:"},{"line_number":122,"context_line":"                self._vault_kv_version \u003d _resp[\u0027data\u0027][\u0027options\u0027][\u0027version\u0027]"},{"line_number":123,"context_line":"            except (KeyError, TypeError):"},{"line_number":124,"context_line":"                # NOTE(e0ne): fallback to v2 since it\u0027s used by default now"},{"line_number":125,"context_line":"                # if vesrion discovery API doesn\u0027t work."},{"line_number":126,"context_line":"                self._vault_kv_version \u003d \u00272\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_72cb9f52","line":123,"range":{"start_line":123,"start_character":30,"end_line":123,"end_character":39},"in_reply_to":"bf51134e_7c2418c7","updated":"2020-06-17 15:31:37.000000000","message":"++","commit_id":"427682081e0f722cdda6a2da1ad5db802ee2e8f1"}]}
