)]}'
{"ironic/conf/redfish.py":[{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"3ac77a60d861e63bdb3b45a6f5dc963630620394","unresolved":false,"context_lines":[{"line_number":29,"context_line":"               help\u003d_(\u0027Number of seconds to wait between attempts to \u0027"},{"line_number":30,"context_line":"                      \u0027connect to Redfish\u0027)),"},{"line_number":31,"context_line":"    cfg.IntOpt(\u0027connection_cache_size\u0027,"},{"line_number":32,"context_line":"               min\u003d0,"},{"line_number":33,"context_line":"               default\u003d1000,"},{"line_number":34,"context_line":"               help\u003d_(\u0027Maximum Redfish client connection cache size. \u0027"},{"line_number":35,"context_line":"                      \u0027Redfish driver would strive to reuse authenticated \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_cb1272b3","line":32,"updated":"2018-11-11 03:04:23.000000000","message":"Probably worth adding a test to ensure the code handles a zero value the way we expect it to.","commit_id":"344466498f753a9863bc964c50b1fe97598974b0"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"3b0754315d9b790365c5339ebf7a214f849e8ded","unresolved":false,"context_lines":[{"line_number":29,"context_line":"               help\u003d_(\u0027Number of seconds to wait between attempts to \u0027"},{"line_number":30,"context_line":"                      \u0027connect to Redfish\u0027)),"},{"line_number":31,"context_line":"    cfg.IntOpt(\u0027connection_cache_size\u0027,"},{"line_number":32,"context_line":"               min\u003d0,"},{"line_number":33,"context_line":"               default\u003d1000,"},{"line_number":34,"context_line":"               help\u003d_(\u0027Maximum Redfish client connection cache size. \u0027"},{"line_number":35,"context_line":"                      \u0027Redfish driver would strive to reuse authenticated \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_dccc6f60","line":32,"in_reply_to":"3f79a3b5_cb1272b3","updated":"2018-11-12 10:02:23.000000000","message":"Done","commit_id":"344466498f753a9863bc964c50b1fe97598974b0"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"d10c22e6031acabc300a55c2ce0e335c7e0a68df","unresolved":false,"context_lines":[{"line_number":29,"context_line":"               help\u003d_(\u0027Number of seconds to wait between attempts to \u0027"},{"line_number":30,"context_line":"                      \u0027connect to Redfish\u0027)),"},{"line_number":31,"context_line":"    cfg.IntOpt(\u0027connection_cache_size\u0027,"},{"line_number":32,"context_line":"               min\u003d0,"},{"line_number":33,"context_line":"               default\u003d1000,"},{"line_number":34,"context_line":"               help\u003d_(\u0027Maximum Redfish client connection cache size. \u0027"},{"line_number":35,"context_line":"                      \u0027Redfish driver would strive to reuse authenticated \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_fd183b88","line":32,"range":{"start_line":32,"start_character":15,"end_line":32,"end_character":21},"updated":"2018-11-13 04:59:57.000000000","message":"May be better to mention it explicitly in the \u0027help\u0027 text that setting the value of \u00270\u0027 would disable the caching of connections.","commit_id":"9e979146392c273f313467648f09c1189219a13e"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"3dfce39199d227ef5a60959bf266e61d377963b2","unresolved":false,"context_lines":[{"line_number":29,"context_line":"               help\u003d_(\u0027Number of seconds to wait between attempts to \u0027"},{"line_number":30,"context_line":"                      \u0027connect to Redfish\u0027)),"},{"line_number":31,"context_line":"    cfg.IntOpt(\u0027connection_cache_size\u0027,"},{"line_number":32,"context_line":"               min\u003d0,"},{"line_number":33,"context_line":"               default\u003d1000,"},{"line_number":34,"context_line":"               help\u003d_(\u0027Maximum Redfish client connection cache size. \u0027"},{"line_number":35,"context_line":"                      \u0027Redfish driver would strive to reuse authenticated \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_03a358ff","line":32,"range":{"start_line":32,"start_character":15,"end_line":32,"end_character":21},"in_reply_to":"3f79a3b5_fd183b88","updated":"2018-11-13 07:32:39.000000000","message":"Done","commit_id":"9e979146392c273f313467648f09c1189219a13e"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"a9dddf9eb4695b23723447c63ad1c92feeb1fc61","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                      \u0027Redfish driver would strive to reuse authenticated \u0027"},{"line_number":36,"context_line":"                      \u0027BMC connections (obtained through Redfish Session \u0027"},{"line_number":37,"context_line":"                      \u0027Service). This option caps the maximum number of \u0027"},{"line_number":38,"context_line":"                      \u0027connections to maintain. The value of `0` disables\u0027"},{"line_number":39,"context_line":"                      \u0027connections caching completely.\u0027))"},{"line_number":40,"context_line":"]"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_188abebc","line":38,"range":{"start_line":38,"start_character":73,"end_line":38,"end_character":74},"updated":"2018-11-15 06:21:50.000000000","message":"Add a space (\u0027 \u0027) at the end.","commit_id":"581ad960170ebc8b4f158aeb2a1a2f3f728faaec"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"b58a8a0ea844151d4fca188c7a9b72f91b7eaead","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                      \u0027Redfish driver would strive to reuse authenticated \u0027"},{"line_number":36,"context_line":"                      \u0027BMC connections (obtained through Redfish Session \u0027"},{"line_number":37,"context_line":"                      \u0027Service). This option caps the maximum number of \u0027"},{"line_number":38,"context_line":"                      \u0027connections to maintain. The value of `0` disables\u0027"},{"line_number":39,"context_line":"                      \u0027connections caching completely.\u0027))"},{"line_number":40,"context_line":"]"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_73de733e","line":38,"range":{"start_line":38,"start_character":73,"end_line":38,"end_character":74},"in_reply_to":"3f79a3b5_188abebc","updated":"2018-11-15 07:25:43.000000000","message":"Done","commit_id":"581ad960170ebc8b4f158aeb2a1a2f3f728faaec"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"a9dddf9eb4695b23723447c63ad1c92feeb1fc61","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                      \u0027BMC connections (obtained through Redfish Session \u0027"},{"line_number":37,"context_line":"                      \u0027Service). This option caps the maximum number of \u0027"},{"line_number":38,"context_line":"                      \u0027connections to maintain. The value of `0` disables\u0027"},{"line_number":39,"context_line":"                      \u0027connections caching completely.\u0027))"},{"line_number":40,"context_line":"]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_b8b4ea82","line":39,"range":{"start_line":39,"start_character":23,"end_line":39,"end_character":34},"updated":"2018-11-15 06:21:50.000000000","message":"s/ connections / client connection /","commit_id":"581ad960170ebc8b4f158aeb2a1a2f3f728faaec"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"b58a8a0ea844151d4fca188c7a9b72f91b7eaead","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                      \u0027BMC connections (obtained through Redfish Session \u0027"},{"line_number":37,"context_line":"                      \u0027Service). This option caps the maximum number of \u0027"},{"line_number":38,"context_line":"                      \u0027connections to maintain. The value of `0` disables\u0027"},{"line_number":39,"context_line":"                      \u0027connections caching completely.\u0027))"},{"line_number":40,"context_line":"]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_93db0f4b","line":39,"range":{"start_line":39,"start_character":23,"end_line":39,"end_character":34},"in_reply_to":"3f79a3b5_b8b4ea82","updated":"2018-11-15 07:25:43.000000000","message":"Done","commit_id":"581ad960170ebc8b4f158aeb2a1a2f3f728faaec"}],"ironic/drivers/modules/redfish/utils.py":[{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"6c5c41152cd8d07c310fd2ddcc492582573d57a9","unresolved":false,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"class SessionCache(object):"},{"line_number":154,"context_line":"    \"\"\"Cache of HTTP sessions credentials\"\"\""},{"line_number":155,"context_line":"    MAX_SESSIONS \u003d CONF.redfish.connection_cache_size"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    _sessions \u003d collections.OrderedDict()"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_8b2f9af2","line":155,"updated":"2018-11-11 02:58:32.000000000","message":"This won\u0027t work, or at the very least is not advised. The problem is that this is evaluated when the file is first loaded, and if that happens before the config is loaded, it will always fallback to the default value.\n\nThis needs to be set in __init__ or re-evaluated everytime.","commit_id":"344466498f753a9863bc964c50b1fe97598974b0"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"3b0754315d9b790365c5339ebf7a214f849e8ded","unresolved":false,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"class SessionCache(object):"},{"line_number":154,"context_line":"    \"\"\"Cache of HTTP sessions credentials\"\"\""},{"line_number":155,"context_line":"    MAX_SESSIONS \u003d CONF.redfish.connection_cache_size"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    _sessions \u003d collections.OrderedDict()"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_111bb083","line":155,"in_reply_to":"3f79a3b5_8b2f9af2","updated":"2018-11-12 10:02:23.000000000","message":"Good catch! Fixed.","commit_id":"344466498f753a9863bc964c50b1fe97598974b0"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"d10c22e6031acabc300a55c2ce0e335c7e0a68df","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __enter__(self):"},{"line_number":165,"context_line":"        try:"},{"line_number":166,"context_line":"            return self.__class__._sessions[self._session_key]"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        except KeyError:"},{"line_number":169,"context_line":"            conn \u003d sushy.Sushy("}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_fd011b0a","line":166,"range":{"start_line":166,"start_character":12,"end_line":166,"end_character":62},"updated":"2018-11-13 04:59:57.000000000","message":"This would throw the KeyError everytime when config connection_cache_size is set to \u00270\u0027. I think you implemented this to avoid multiple sushy connection creation call.\nCan we instead:-\nif CONF.redfish.connection_cache_size \u003d\u003d 0:\n    return sushy.Sushy( ... )\ntry:\n    return self.__class__._sessions[self._session_key]\nexcept KeyError:\n    conn \u003d sushy.Sushy( ....)\n    self.__class__._sessions[self._session_key] \u003d conn\n    if (len(self.__class__._sessions) \u003e\n            CONF.redfish.connection_cache_size):\n            self._expire_oldest_session()\n    return conn","commit_id":"9e979146392c273f313467648f09c1189219a13e"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"3dfce39199d227ef5a60959bf266e61d377963b2","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def __enter__(self):"},{"line_number":165,"context_line":"        try:"},{"line_number":166,"context_line":"            return self.__class__._sessions[self._session_key]"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        except KeyError:"},{"line_number":169,"context_line":"            conn \u003d sushy.Sushy("}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_038cb87f","line":166,"range":{"start_line":166,"start_character":12,"end_line":166,"end_character":62},"in_reply_to":"3f79a3b5_fd011b0a","updated":"2018-11-13 07:32:39.000000000","message":"I am not sure that would read better...\n\nOne thing is that we would have to instantiate `Sushy` at two places. We could work around this slight duplication by localizing it to a function or partial, but would not that be a unnecessary complication?\n\nThe other thing that Python seems to promote \"asking for forgiveness instead of permission\" approach [1][2]. Doing explicit check for enabled connections caching, as opposed to catching exception,  might read as asking for permission...\n\nFinally, from the performance standpoint, it seems that if we\u0027d have caching enabled by default, the comparison (connection_cache_size \u003d\u003d 0) would yield False most of the times....\n\nWDYT?\n\n1. https://docs.quantifiedcode.com/python-anti-patterns/readability/asking_for_permission_instead_of_forgiveness_when_working_with_files.html\n2. https://stackoverflow.com/questions/12265451/ask-forgiveness-not-permission-explain","commit_id":"9e979146392c273f313467648f09c1189219a13e"}]}