)]}'
{"nova/tests/unit/virt/xenapi/client/test_session.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"98e514c203076f0a805f4aa2709027a03e21c6a8","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        session.XenAPISession(\u0027http://slave\u0027, \u0027username\u0027, \u0027password\u0027)"},{"line_number":89,"context_line":"        self.assertEqual(3, sess.login_with_password.call_count)"},{"line_number":90,"context_line":"        self.assertEqual(3, mock_timeout.call_count)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"class ApplySessionHelpersTestCase(stubs.XenAPITestBaseNoDB):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_909267f4","line":90,"updated":"2016-08-24 10:19:06.000000000","message":"I don\u0027t see a unit test for the new code, this just seems to make the existing tests pass?","commit_id":"6b9736497b6087855e34dca84fad97372def09f9"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"0a8f00b2f5d1eae3ff5ff055170e7cea92661671","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        session.XenAPISession(\u0027http://slave\u0027, \u0027username\u0027, \u0027password\u0027)"},{"line_number":89,"context_line":"        self.assertEqual(3, sess.login_with_password.call_count)"},{"line_number":90,"context_line":"        self.assertEqual(3, mock_timeout.call_count)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"class ApplySessionHelpersTestCase(stubs.XenAPITestBaseNoDB):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_d3918948","line":90,"in_reply_to":"1ac06dbe_909267f4","updated":"2016-08-24 10:32:08.000000000","message":"Yes, if new code added, I should add unit test code","commit_id":"6b9736497b6087855e34dca84fad97372def09f9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"66081c6b01401bc3db8d43457af6da15815a1fa5","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":107,"context_line":"                                \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"},{"line_number":108,"context_line":"        self.assertTrue(xenapi_sess._get_relax_xsm_sr_check())"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027call_plugin\u0027)"},{"line_number":111,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_e1e4c40e","line":108,"updated":"2016-08-26 10:08:02.000000000","message":"Use self.assertEqual(True, ...)","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"35754ebc91ba7b9af10209a9c8a8dad1ea93686e","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":107,"context_line":"                                \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"},{"line_number":108,"context_line":"        self.assertTrue(xenapi_sess._get_relax_xsm_sr_check())"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027call_plugin\u0027)"},{"line_number":111,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_87301da0","line":108,"in_reply_to":"fa7ab95a_e1e4c40e","updated":"2016-08-29 01:19:35.000000000","message":"Done","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"66081c6b01401bc3db8d43457af6da15815a1fa5","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":126,"context_line":"                            \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"},{"line_number":127,"context_line":"        self.assertTrue(not xenapi_sess._get_relax_xsm_sr_check())"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027call_plugin\u0027)"},{"line_number":130,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_21c6ac67","line":127,"updated":"2016-08-26 10:08:02.000000000","message":"This should assert that it\u0027s False, not assert that the negative is True...\n\nIf get_relax_xsm_sr_check returned None, then \"not None\" is True.","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"35754ebc91ba7b9af10209a9c8a8dad1ea93686e","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":126,"context_line":"                            \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"},{"line_number":127,"context_line":"        self.assertTrue(not xenapi_sess._get_relax_xsm_sr_check())"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027call_plugin\u0027)"},{"line_number":130,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_a727e162","line":127,"in_reply_to":"fa7ab95a_21c6ac67","updated":"2016-08-29 01:19:35.000000000","message":"agreed, will change it","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"867dd04fca24d99b68fb452dbbd8513c318bd3b3","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        mock_product_version_and_brand.return_value \u003d (7, 0, 0), \u0027XenServer\u0027"},{"line_number":171,"context_line":"        mock_platform_version.return_value \u003d (2, 1, 0)"},{"line_number":172,"context_line":"        # mark no relax-xsm-sr-check setting in /etc/xapi.conf"},{"line_number":173,"context_line":"        mock_call_plugin.side_effect \u003d Exception(\u0027No relax-xsm-sr-check set\u0027)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":176,"context_line":"                            \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a8bc5a2_4bd0ad1f","line":173,"updated":"2016-09-26 12:40:23.000000000","message":"Does it raise an exception?  Looking at the code at https://git.openstack.org/cgit/openstack/nova/tree/plugins/xenserver/xenapi/etc/xapi.d/plugins/config_file.py it seems to just return \"\"?","commit_id":"cf2894d9588cbf92a5211ed71b4cdde4894b8631"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"cf5611f710361db9d95f45c239608b8a7c4ceb8f","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        mock_product_version_and_brand.return_value \u003d (7, 0, 0), \u0027XenServer\u0027"},{"line_number":171,"context_line":"        mock_platform_version.return_value \u003d (2, 1, 0)"},{"line_number":172,"context_line":"        # mark no relax-xsm-sr-check setting in /etc/xapi.conf"},{"line_number":173,"context_line":"        mock_call_plugin.side_effect \u003d Exception(\u0027No relax-xsm-sr-check set\u0027)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":176,"context_line":"                            \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a95cdbc_976d5cce","line":173,"in_reply_to":"5a8bc5a2_4bd0ad1f","updated":"2016-10-08 11:44:21.000000000","message":"no exception raised, I will change this","commit_id":"cf2894d9588cbf92a5211ed71b4cdde4894b8631"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"d06ff855e3f95d193407abc8e8ca6b13233163b9","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"},{"line_number":116,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_verify_plugin_version\u0027)"},{"line_number":117,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_create_session\u0027)"},{"line_number":118,"context_line":"    def test_relax_xsm_sr_check_XS65_False(self, mock_create_session,"},{"line_number":119,"context_line":"                                        mock_verify_plugin_version,"},{"line_number":120,"context_line":"                                        mock_product_version_and_brand,"},{"line_number":121,"context_line":"                                        mock_platform_version,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_8134274a","line":118,"updated":"2016-10-10 08:30:19.000000000","message":"Perhaps test_relax_xsm_sr_check_XS65_missing so it\u0027s clear what the test is testing?","commit_id":"f3a7ef02eae85ccec84522ec1997671a1f71e72c"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"af96a6454cdfa63d42af07c12f90a1f37a9d24cd","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"},{"line_number":116,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_verify_plugin_version\u0027)"},{"line_number":117,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_create_session\u0027)"},{"line_number":118,"context_line":"    def test_relax_xsm_sr_check_XS65_False(self, mock_create_session,"},{"line_number":119,"context_line":"                                        mock_verify_plugin_version,"},{"line_number":120,"context_line":"                                        mock_product_version_and_brand,"},{"line_number":121,"context_line":"                                        mock_platform_version,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_f4768704","line":118,"in_reply_to":"1a95cdbc_8134274a","updated":"2016-10-10 08:55:07.000000000","message":"Done","commit_id":"f3a7ef02eae85ccec84522ec1997671a1f71e72c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"d06ff855e3f95d193407abc8e8ca6b13233163b9","unresolved":false,"context_lines":[{"line_number":137,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"},{"line_number":138,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_verify_plugin_version\u0027)"},{"line_number":139,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_create_session\u0027)"},{"line_number":140,"context_line":"    def test_relax_xsm_sr_check_XS7_True(self, mock_create_session,"},{"line_number":141,"context_line":"                                        mock_verify_plugin_version,"},{"line_number":142,"context_line":"                                        mock_product_version_and_brand,"},{"line_number":143,"context_line":"                                        mock_platform_version,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_4115bf99","line":140,"updated":"2016-10-10 08:30:19.000000000","message":"Method name doesn\u0027t match with either comment @150 or code @151","commit_id":"f3a7ef02eae85ccec84522ec1997671a1f71e72c"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"af96a6454cdfa63d42af07c12f90a1f37a9d24cd","unresolved":false,"context_lines":[{"line_number":137,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_get_product_version_and_brand\u0027)"},{"line_number":138,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_verify_plugin_version\u0027)"},{"line_number":139,"context_line":"    @mock.patch.object(session.XenAPISession, \u0027_create_session\u0027)"},{"line_number":140,"context_line":"    def test_relax_xsm_sr_check_XS7_True(self, mock_create_session,"},{"line_number":141,"context_line":"                                        mock_verify_plugin_version,"},{"line_number":142,"context_line":"                                        mock_product_version_and_brand,"},{"line_number":143,"context_line":"                                        mock_platform_version,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_d4526b66","line":140,"in_reply_to":"1a95cdbc_4115bf99","updated":"2016-10-10 08:55:07.000000000","message":"sorry, I made some change but forgot to change the comments, will modify","commit_id":"f3a7ef02eae85ccec84522ec1997671a1f71e72c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"d06ff855e3f95d193407abc8e8ca6b13233163b9","unresolved":false,"context_lines":[{"line_number":148,"context_line":"        mock_product_version_and_brand.return_value \u003d (7, 0, 0), \u0027XenServer\u0027"},{"line_number":149,"context_line":"        mock_platform_version.return_value \u003d (2, 1, 0)"},{"line_number":150,"context_line":"        # mark relax-xsm-sr-check\u003dFalse in /etc/xapi.conf"},{"line_number":151,"context_line":"        mock_call_plugin.return_value \u003d \"\""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":154,"context_line":"                            \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_412e9ffb","line":151,"updated":"2016-10-10 08:30:19.000000000","message":"This should return False base on the previous comment?","commit_id":"f3a7ef02eae85ccec84522ec1997671a1f71e72c"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"af96a6454cdfa63d42af07c12f90a1f37a9d24cd","unresolved":false,"context_lines":[{"line_number":148,"context_line":"        mock_product_version_and_brand.return_value \u003d (7, 0, 0), \u0027XenServer\u0027"},{"line_number":149,"context_line":"        mock_platform_version.return_value \u003d (2, 1, 0)"},{"line_number":150,"context_line":"        # mark relax-xsm-sr-check\u003dFalse in /etc/xapi.conf"},{"line_number":151,"context_line":"        mock_call_plugin.return_value \u003d \"\""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        xenapi_sess \u003d session.XenAPISession("},{"line_number":154,"context_line":"                            \u0027http://someserver\u0027, \u0027username\u0027, \u0027password\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_144933b7","line":151,"in_reply_to":"1a95cdbc_412e9ffb","updated":"2016-10-10 08:55:07.000000000","message":"it\u0027s my bad, will modify it","commit_id":"f3a7ef02eae85ccec84522ec1997671a1f71e72c"}],"nova/virt/xenapi/client/session.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"98e514c203076f0a805f4aa2709027a03e21c6a8","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        # not in others (e.g. xenserver-core, XAPI-XCP)."},{"line_number":161,"context_line":"        # In these cases, the platform version is the best number to use."},{"line_number":162,"context_line":"        if product_version_str is None:"},{"line_number":163,"context_line":"            product_version_str \u003d software_version.get(\u0027platform_version\u0027,"},{"line_number":164,"context_line":"                                                       \u00270.0.0\u0027)"},{"line_number":165,"context_line":"        product_brand \u003d software_version.get(\u0027product_brand\u0027)"},{"line_number":166,"context_line":"        product_version \u003d versionutils.convert_version_to_tuple("}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_b0e203d3","line":163,"updated":"2016-08-24 10:19:06.000000000","message":"Hmm, this seems to duplicate the logic here?\n\nMaybe always return the plaform_version from here?","commit_id":"6b9736497b6087855e34dca84fad97372def09f9"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"0a8f00b2f5d1eae3ff5ff055170e7cea92661671","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        # not in others (e.g. xenserver-core, XAPI-XCP)."},{"line_number":161,"context_line":"        # In these cases, the platform version is the best number to use."},{"line_number":162,"context_line":"        if product_version_str is None:"},{"line_number":163,"context_line":"            product_version_str \u003d software_version.get(\u0027platform_version\u0027,"},{"line_number":164,"context_line":"                                                       \u00270.0.0\u0027)"},{"line_number":165,"context_line":"        product_brand \u003d software_version.get(\u0027product_brand\u0027)"},{"line_number":166,"context_line":"        product_version \u003d versionutils.convert_version_to_tuple("}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_13aa0185","line":163,"in_reply_to":"1ac06dbe_b0e203d3","updated":"2016-08-24 10:32:08.000000000","message":"yes, I also want to make this function always return platform_version, but not sure whether I can. Also, if we always return platform_version, need to change other related code, see:https://github.com/openstack/nova/blob/a0b224b17b34c4c1879362e95633f0ccf35eb502/nova/virt/xenapi/vm_utils.py#L160 https://github.com/openstack/nova/blob/a0b224b17b34c4c1879362e95633f0ccf35eb502/nova/virt/xenapi/vm_utils.py#L792","commit_id":"6b9736497b6087855e34dca84fad97372def09f9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"66081c6b01401bc3db8d43457af6da15815a1fa5","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.product_version, self.product_brand \u003d \\"},{"line_number":95,"context_line":"            self._get_product_version_and_brand()"},{"line_number":96,"context_line":"        self._verify_plugin_version()"},{"line_number":97,"context_line":"        self._cached_xsm_sr_relaxed \u003d self._get_relax_xsm_sr_check()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        apply_session_helpers(self)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_def5e183","line":97,"updated":"2016-08-26 10:08:02.000000000","message":"Set to None","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"35754ebc91ba7b9af10209a9c8a8dad1ea93686e","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.product_version, self.product_brand \u003d \\"},{"line_number":95,"context_line":"            self._get_product_version_and_brand()"},{"line_number":96,"context_line":"        self._verify_plugin_version()"},{"line_number":97,"context_line":"        self._cached_xsm_sr_relaxed \u003d self._get_relax_xsm_sr_check()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        apply_session_helpers(self)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_c7e14500","line":97,"in_reply_to":"fa7ab95a_def5e183","updated":"2016-08-29 01:19:35.000000000","message":"Done","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"66081c6b01401bc3db8d43457af6da15815a1fa5","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        # Product version is only set in some cases (e.g. XCP, XenServer) and"},{"line_number":159,"context_line":"        # not in others (e.g. xenserver-core, XAPI-XCP)."},{"line_number":160,"context_line":"        # In these cases, the platform version is the best number to use."},{"line_number":161,"context_line":"        product_version_str \u003d software_version.get(\u0027platform_version\u0027,"},{"line_number":162,"context_line":"                                                   \u00270.0.0\u0027)"},{"line_number":163,"context_line":"        product_brand \u003d software_version.get(\u0027product_brand\u0027)"},{"line_number":164,"context_line":"        product_version \u003d versionutils.convert_version_to_tuple("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_fead4589","line":161,"updated":"2016-08-26 10:08:02.000000000","message":"Perhaps we should use platform_version moving forward, but if we\u0027re going to stop using product_version then that would need to be in a separate change.\n\nI think we have two options:\n1) Extract out the change to use platform_version into a new change or\n2) add platform_version as a new value and use that for this test.","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"35754ebc91ba7b9af10209a9c8a8dad1ea93686e","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        # Product version is only set in some cases (e.g. XCP, XenServer) and"},{"line_number":159,"context_line":"        # not in others (e.g. xenserver-core, XAPI-XCP)."},{"line_number":160,"context_line":"        # In these cases, the platform version is the best number to use."},{"line_number":161,"context_line":"        product_version_str \u003d software_version.get(\u0027platform_version\u0027,"},{"line_number":162,"context_line":"                                                   \u00270.0.0\u0027)"},{"line_number":163,"context_line":"        product_brand \u003d software_version.get(\u0027product_brand\u0027)"},{"line_number":164,"context_line":"        product_version \u003d versionutils.convert_version_to_tuple("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_27f3d1d7","line":161,"in_reply_to":"fa7ab95a_fead4589","updated":"2016-08-29 01:19:35.000000000","message":"Let\u0027s create a new function and avoid change the existing function","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"66081c6b01401bc3db8d43457af6da15815a1fa5","unresolved":false,"context_lines":[{"line_number":349,"context_line":"            conn.close()"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def is_xsm_sr_check_relaxed(self):"},{"line_number":352,"context_line":"        return self._cached_xsm_sr_relaxed"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _get_relax_xsm_sr_check(self):"},{"line_number":355,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_be549d54","line":352,"updated":"2016-08-26 10:08:02.000000000","message":"I think the pattern should be: \n\nif not self._cached_xsm_sr_relaxed:\n   include the code for _get_relax_xsm_sr_check (rather than calling it)\nreturn self._cached_xsm_sr_relaxed","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"35754ebc91ba7b9af10209a9c8a8dad1ea93686e","unresolved":false,"context_lines":[{"line_number":349,"context_line":"            conn.close()"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def is_xsm_sr_check_relaxed(self):"},{"line_number":352,"context_line":"        return self._cached_xsm_sr_relaxed"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def _get_relax_xsm_sr_check(self):"},{"line_number":355,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_07cb2d73","line":352,"in_reply_to":"fa7ab95a_be549d54","updated":"2016-08-29 01:19:35.000000000","message":"Done","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"66081c6b01401bc3db8d43457af6da15815a1fa5","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            # platform major version is equal/greater than 2 since XS7.0"},{"line_number":362,"context_line":"            version_str \u003d \u0027.\u0027.join("},{"line_number":363,"context_line":"                    str(v) for v in self.product_version)"},{"line_number":364,"context_line":"            if versionutils.is_compatible(\u00272.1.0\u0027, version_str):"},{"line_number":365,"context_line":"                config_value \u003d \u0027true\u0027"},{"line_number":366,"context_line":"            else:"},{"line_number":367,"context_line":"                config_value \u003d \u0027false\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_fee545a8","line":364,"updated":"2016-08-26 10:08:02.000000000","message":"I\u0027d rather see this moved to before the try/except block, setting default_config_value, which is then returned by the exception block","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"35754ebc91ba7b9af10209a9c8a8dad1ea93686e","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            # platform major version is equal/greater than 2 since XS7.0"},{"line_number":362,"context_line":"            version_str \u003d \u0027.\u0027.join("},{"line_number":363,"context_line":"                    str(v) for v in self.product_version)"},{"line_number":364,"context_line":"            if versionutils.is_compatible(\u00272.1.0\u0027, version_str):"},{"line_number":365,"context_line":"                config_value \u003d \u0027true\u0027"},{"line_number":366,"context_line":"            else:"},{"line_number":367,"context_line":"                config_value \u003d \u0027false\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_27caf16d","line":364,"in_reply_to":"fa7ab95a_fee545a8","updated":"2016-08-29 01:19:35.000000000","message":"Done","commit_id":"36ddc4e774eb80377160c0343e5a500f282a728e"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"23b54f8429c1a393e07151afa19519cecd4a725f","unresolved":false,"context_lines":[{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def is_xsm_sr_check_relaxed(self):"},{"line_number":364,"context_line":"        if self._cached_xsm_sr_relaxed is None:"},{"line_number":365,"context_line":"            config_value \u003d \u0027false\u0027"},{"line_number":366,"context_line":"            version_str \u003d \u0027.\u0027.join(str(v) for v in self.platform_version)"},{"line_number":367,"context_line":"            if versionutils.is_compatible(\u00272.1.0\u0027, version_str):"},{"line_number":368,"context_line":"                config_value \u003d \u0027true\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_b69ed504","line":365,"updated":"2016-08-30 11:01:02.000000000","message":"This seems to be correct, but the logic is a little harder than I would prefer.\nAlso, what happens if the next release of XS is platform 3.x? We should assume, unless otherwise stated, that the default will not change.\n\nCould we have something like the following?\ndefault_config_value \u003d FALSE if 1.9, TRUE if \u003e 1.9\ntry:\n    config_value \u003d self.call_plugin(...).lower() \u003d\u003d \u0027true\u0027\nexcept Exception:\n    config_value \u003d default_config_value\n\n?","commit_id":"3d74ea8f98128a131f8cffb40a6b522b20ae5938"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"8f0cf97abb992c76e5be3677017838dbde0a810d","unresolved":false,"context_lines":[{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def is_xsm_sr_check_relaxed(self):"},{"line_number":364,"context_line":"        if self._cached_xsm_sr_relaxed is None:"},{"line_number":365,"context_line":"            config_value \u003d \u0027false\u0027"},{"line_number":366,"context_line":"            version_str \u003d \u0027.\u0027.join(str(v) for v in self.platform_version)"},{"line_number":367,"context_line":"            if versionutils.is_compatible(\u00272.1.0\u0027, version_str):"},{"line_number":368,"context_line":"                config_value \u003d \u0027true\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_3cda71ea","line":365,"in_reply_to":"fa7ab95a_b69ed504","updated":"2016-08-31 03:01:17.000000000","message":"I found I can set same_major\u003dFalse, to compare the compatibility, oslo.versionutils.is_compatible(requested_version, current_version, same_major\u003dTrue)","commit_id":"3d74ea8f98128a131f8cffb40a6b522b20ae5938"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"3b51f05412646d52e7a04a4a2ddc3f1df14b6cd5","unresolved":false,"context_lines":[{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def is_xsm_sr_check_relaxed(self):"},{"line_number":364,"context_line":"        if self._cached_xsm_sr_relaxed is None:"},{"line_number":365,"context_line":"            config_value \u003d \u0027false\u0027"},{"line_number":366,"context_line":"            version_str \u003d \u0027.\u0027.join(str(v) for v in self.platform_version)"},{"line_number":367,"context_line":"            if versionutils.is_compatible(\u00272.1.0\u0027, version_str):"},{"line_number":368,"context_line":"                config_value \u003d \u0027true\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_31945a43","line":365,"in_reply_to":"fa7ab95a_b69ed504","updated":"2016-08-31 01:51:19.000000000","message":"Sorry that I mis-understood the versionutils.is_compatible(), I thought 3.x will also compatible with \u00272.1.0\u0027 in the function is_compatible(), but I\u0027m wrong, will change this part","commit_id":"3d74ea8f98128a131f8cffb40a6b522b20ae5938"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"867dd04fca24d99b68fb452dbbd8513c318bd3b3","unresolved":false,"context_lines":[{"line_number":392,"context_line":"                                    \u0027get_val\u0027,"},{"line_number":393,"context_line":"                                    key\u003d\u0027relax-xsm-sr-check\u0027).lower() \u003d\u003d \u0027true\u0027"},{"line_number":394,"context_line":"            except Exception:"},{"line_number":395,"context_line":"                self._cached_xsm_sr_relaxed \u003d default_config_value"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"        return self._cached_xsm_sr_relaxed"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a8bc5a2_8b60f5d0","line":395,"updated":"2016-09-26 12:40:23.000000000","message":"This exception should probably be logged (but at debug level?) and a note added to the log file that we\u0027re just using the default.\nAlso be more restrictive about using Exception; can\u0027t we use a tighter catch here?\n\nAlso, where is the exception actually raised?  https://git.openstack.org/cgit/openstack/nova/tree/plugins/xenserver/xenapi/etc/xapi.d/plugins/config_file.py seems to just return the empty string, so would this actually work?","commit_id":"cf2894d9588cbf92a5211ed71b4cdde4894b8631"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"cf5611f710361db9d95f45c239608b8a7c4ceb8f","unresolved":false,"context_lines":[{"line_number":392,"context_line":"                                    \u0027get_val\u0027,"},{"line_number":393,"context_line":"                                    key\u003d\u0027relax-xsm-sr-check\u0027).lower() \u003d\u003d \u0027true\u0027"},{"line_number":394,"context_line":"            except Exception:"},{"line_number":395,"context_line":"                self._cached_xsm_sr_relaxed \u003d default_config_value"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"        return self._cached_xsm_sr_relaxed"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a95cdbc_5777549f","line":395,"in_reply_to":"5a8bc5a2_8b60f5d0","updated":"2016-10-08 11:44:21.000000000","message":"yes, there is no exception, I will change the code, thanks for so detailed review:)","commit_id":"cf2894d9588cbf92a5211ed71b4cdde4894b8631"},{"author":{"_account_id":17169,"name":"John Hua","email":"ihuazhihao@gmail.com","username":"johnhua"},"change_message_id":"8e2d1be5800acb75cab1fa832528751a1300cb17","unresolved":false,"context_lines":[{"line_number":381,"context_line":"            config_value \u003d self.call_plugin(\u0027config_file\u0027, \u0027get_val\u0027,"},{"line_number":382,"context_line":"                                            key\u003d\u0027relax-xsm-sr-check\u0027)"},{"line_number":383,"context_line":"            if not config_value:"},{"line_number":384,"context_line":"                version_str \u003d \u0027.\u0027.join(str(v) for v in self.platform_version)"},{"line_number":385,"context_line":"                if versionutils.is_compatible(\u00272.1.0\u0027, version_str,"},{"line_number":386,"context_line":"                                              same_major\u003dFalse):"},{"line_number":387,"context_line":"                    self._cached_xsm_sr_relaxed \u003d True"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_e58daf7c","line":384,"range":{"start_line":384,"start_character":30,"end_line":384,"end_character":77},"updated":"2016-10-11 10:08:07.000000000","message":"Perhaps \u0027.\u0027.join(map(str,self.platform_version)) but basically the same","commit_id":"8057cab9f66a02a54a38ca4aa2baa0b05e06dac3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"57534fa9e9b14e45fa5a3fc71c77e5abbdd880b4","unresolved":false,"context_lines":[{"line_number":377,"context_line":"            conn.close()"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def is_xsm_sr_check_relaxed(self):"},{"line_number":380,"context_line":"        if self._cached_xsm_sr_relaxed is None:"},{"line_number":381,"context_line":"            config_value \u003d self.call_plugin(\u0027config_file\u0027, \u0027get_val\u0027,"},{"line_number":382,"context_line":"                                            key\u003d\u0027relax-xsm-sr-check\u0027)"},{"line_number":383,"context_line":"            if not config_value:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa6399be_5105d30e","line":380,"updated":"2016-10-20 10:04:51.000000000","message":"Nit: I am not keen on how caching this and change it are in the same patch.","commit_id":"22b8ae0975187a847fc201a750f05be0af9818a4"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"98e514c203076f0a805f4aa2709027a03e21c6a8","unresolved":false,"context_lines":[{"line_number":2227,"context_line":"                                                      key\u003d\u0027relax-xsm-sr-check\u0027)"},{"line_number":2228,"context_line":"            except Exception:"},{"line_number":2229,"context_line":"                # platform major version is equal/greater than 2 since XS7.0"},{"line_number":2230,"context_line":"                version_str \u003d \u0027.\u0027.join("},{"line_number":2231,"context_line":"                                str(v) for v in self._session.platform_version)"},{"line_number":2232,"context_line":"                if versionutils.is_compatible(\u00272.1.0\u0027, version_str):"},{"line_number":2233,"context_line":"                    config_value \u003d \"true\""},{"line_number":2234,"context_line":"                else:"},{"line_number":2235,"context_line":"                    LOG.exception(_LE(\u0027Plugin config_file get_val failed\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_b09a0312","line":2232,"range":{"start_line":2230,"start_character":0,"end_line":2232,"end_character":68},"updated":"2016-08-24 10:19:06.000000000","message":"Could we extract a function called _is_xsm_sr_check_default_true() or something like that?","commit_id":"6b9736497b6087855e34dca84fad97372def09f9"},{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"0a8f00b2f5d1eae3ff5ff055170e7cea92661671","unresolved":false,"context_lines":[{"line_number":2227,"context_line":"                                                      key\u003d\u0027relax-xsm-sr-check\u0027)"},{"line_number":2228,"context_line":"            except Exception:"},{"line_number":2229,"context_line":"                # platform major version is equal/greater than 2 since XS7.0"},{"line_number":2230,"context_line":"                version_str \u003d \u0027.\u0027.join("},{"line_number":2231,"context_line":"                                str(v) for v in self._session.platform_version)"},{"line_number":2232,"context_line":"                if versionutils.is_compatible(\u00272.1.0\u0027, version_str):"},{"line_number":2233,"context_line":"                    config_value \u003d \"true\""},{"line_number":2234,"context_line":"                else:"},{"line_number":2235,"context_line":"                    LOG.exception(_LE(\u0027Plugin config_file get_val failed\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_b304cd14","line":2232,"range":{"start_line":2230,"start_character":0,"end_line":2232,"end_character":68},"in_reply_to":"1ac06dbe_b09a0312","updated":"2016-08-24 10:32:08.000000000","message":"yes, I can extrac one","commit_id":"6b9736497b6087855e34dca84fad97372def09f9"}]}