)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"e563688022c0def49c2e745b290c957275987730","unresolved":true,"context_lines":[{"line_number":7,"context_line":"[SVF]:Fix create volume on drp"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"[Spectrum Virtualize Family] While creating volumes on"},{"line_number":10,"context_line":"data reduction pool, incorrect options were picked."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch fixes the issue by reading the data reduction"},{"line_number":13,"context_line":"pool details correctly during intialization."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"34c17dc9_76e82eba","line":10,"range":{"start_line":10,"start_character":21,"end_line":10,"end_character":50},"updated":"2021-08-10 18:09:05.000000000","message":"My concern is neither the commit message nor the LP bug [1] really convey the root cause of the problem. I think it would be clearer if the commit message stated this patch fixes a bug caused by a case sensitive string comparison that caused the code to incorrectly identify whether DR was enabled in a pool.\n\nI don\u0027t know if this is a nit, but I\u0027ll vote +1 because the code looks good.","commit_id":"942cd7b733d19172d0e53e5cfb460fb156b93456"}],"cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3409e034e91de3b5f356a3222c16a1d43dabf3fc","unresolved":true,"context_lines":[{"line_number":5442,"context_line":"                       \u0027get_pool_attrs\u0027)"},{"line_number":5443,"context_line":"    def test_build_pool_stats_drp(self, is_drp, get_pool_attrs):"},{"line_number":5444,"context_line":"        get_pool_attrs.return_value \u003d {\u0027id\u0027: 1, \u0027name\u0027: \u0027openstack\u0027,"},{"line_number":5445,"context_line":"                                       \u0027data_reduction\u0027: is_drp,"},{"line_number":5446,"context_line":"                                       \u0027easy_tier\u0027: \u0027on\u0027,"},{"line_number":5447,"context_line":"                                       \u0027capacity\u0027: \u002720\u0027,"},{"line_number":5448,"context_line":"                                       \u0027free_capacity\u0027: \u002740\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"cee80526_1d4b77b4","line":5445,"updated":"2021-08-17 14:55:00.000000000","message":"This doesn\u0027t test the case where data_reduction is not present in the dict at all.","commit_id":"e34a22e581084bcad82f5b6e521b46358ebc0fcd"},{"author":{"_account_id":32036,"name":"katari manoj kumar","email":"katkumar@in.ibm.com","username":"katarimanojkumar"},"change_message_id":"7e1146c2df058ef49482ad2ad14121cfe1b3c126","unresolved":true,"context_lines":[{"line_number":5442,"context_line":"                       \u0027get_pool_attrs\u0027)"},{"line_number":5443,"context_line":"    def test_build_pool_stats_drp(self, is_drp, get_pool_attrs):"},{"line_number":5444,"context_line":"        get_pool_attrs.return_value \u003d {\u0027id\u0027: 1, \u0027name\u0027: \u0027openstack\u0027,"},{"line_number":5445,"context_line":"                                       \u0027data_reduction\u0027: is_drp,"},{"line_number":5446,"context_line":"                                       \u0027easy_tier\u0027: \u0027on\u0027,"},{"line_number":5447,"context_line":"                                       \u0027capacity\u0027: \u002720\u0027,"},{"line_number":5448,"context_line":"                                       \u0027free_capacity\u0027: \u002740\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"fc896d23_c3d7c703","line":5445,"in_reply_to":"cee80526_1d4b77b4","updated":"2021-08-17 15:40:31.000000000","message":"Addressed in the latest patchset","commit_id":"e34a22e581084bcad82f5b6e521b46358ebc0fcd"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"104ebc9dc81bfdfd1857cc4267510a6d1f5ef276","unresolved":true,"context_lines":[{"line_number":5436,"context_line":"                except processutils.ProcessExecutionError as e:"},{"line_number":5437,"context_line":"                    if \u0027CMMVC7050E\u0027 not in e.stderr:"},{"line_number":5438,"context_line":"                        raise"},{"line_number":5439,"context_line":""},{"line_number":5440,"context_line":"    @ddt.data((\u0027yes\u0027), (\u0027Yes\u0027), (\u0027no\u0027), (\u0027NO\u0027), (\u0027\u0027))"},{"line_number":5441,"context_line":"    @mock.patch.object(storwize_svc_common.StorwizeHelpers,"},{"line_number":5442,"context_line":"                       \u0027get_pool_attrs\u0027)"},{"line_number":5443,"context_line":"    def test_build_pool_stats_drp(self, is_drp, get_pool_attrs):"},{"line_number":5444,"context_line":"        get_pool_attrs.return_value \u003d {\u0027id\u0027: 1, \u0027name\u0027: \u0027openstack\u0027,"},{"line_number":5445,"context_line":"                                       \u0027data_reduction\u0027: is_drp,"},{"line_number":5446,"context_line":"                                       \u0027easy_tier\u0027: \u0027on\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1b72de58_848fb544","line":5443,"range":{"start_line":5439,"start_character":0,"end_line":5443,"end_character":64},"updated":"2021-08-17 18:22:45.000000000","message":"You don\u0027t have to change it for this patch, but I think it\u0027s better to put the expected response in the data instead of putting conditional logic in the test.  What I mean is, if you set up the test like this:\n\n    @ddt.data((\u0027yes\u0027, True),\n              (\u0027Yes\u0027, True),\n              (\u0027YES\u0027, True),\n              (\u0027no\u0027, False),\n              (\u0027NO\u0027, False),\n              (\u0027\u0027, False))\n    @ddt.unpack\n    @mock.patch.object(storwize_svc_common.StorwizeHelpers,\n                       \u0027get_pool_attrs\u0027)\n    def test_build_pool_stats_drp(self,\n                                  is_drp_response, should_be_drp,\n                                  get_pool_attrs):\n\nthen you can replace lines 5457-5460 with:\n\n        self.assertEqual(should_be_drp, pool_stats[\u0027data_reduction\u0027])\n\nI think it makes it easier to see what\u0027s being tested, plus it\u0027s a lot easier to add more test cases if you want to.","commit_id":"70a07ee7a2fda6c129903a21431fa2d499f4160d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"104ebc9dc81bfdfd1857cc4267510a6d1f5ef276","unresolved":true,"context_lines":[{"line_number":5461,"context_line":""},{"line_number":5462,"context_line":"    @mock.patch.object(storwize_svc_common.StorwizeHelpers,"},{"line_number":5463,"context_line":"                       \u0027get_pool_attrs\u0027)"},{"line_number":5464,"context_line":"    def test_build_pool_stats_drp_none(self, get_pool_attrs):"},{"line_number":5465,"context_line":"        get_pool_attrs.return_value \u003d {\u0027id\u0027: 1, \u0027name\u0027: \u0027openstack1\u0027,"},{"line_number":5466,"context_line":"                                       \u0027easy_tier\u0027: \u0027on\u0027,"},{"line_number":5467,"context_line":"                                       \u0027capacity\u0027: \u002720\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3c53f076_b6339245","line":5464,"updated":"2021-08-17 18:22:45.000000000","message":"Good to see that you addressed this case.","commit_id":"70a07ee7a2fda6c129903a21431fa2d499f4160d"}],"cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b5e1baf0456d1d9a4843f7dba50264bb69b5c0fc","unresolved":true,"context_lines":[{"line_number":5989,"context_line":"            # is_dr_pool flag."},{"line_number":5990,"context_line":"            if pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027yes\u0027:"},{"line_number":5991,"context_line":"                is_dr_pool \u003d True"},{"line_number":5992,"context_line":"            elif pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027no\u0027:"},{"line_number":5993,"context_line":"                is_dr_pool \u003d False"},{"line_number":5994,"context_line":""},{"line_number":5995,"context_line":"            pool_stats \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"09df4553_77247db1","line":5992,"updated":"2021-07-16 13:47:15.000000000","message":"The use of \u0027elif\u0027 suggests neither of the two previous choices may be valid and that is_dr_pool isn\u0027t really a boolean (although I do see the variable is initialized on L5963).\n\nThis form would be much more concise because the right side returns a boolean.\n\n            is_dr_pool \u003d pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027yes\u0027","commit_id":"ff402df67948eee78c48e1f221540fa491b42f0f"},{"author":{"_account_id":32036,"name":"katari manoj kumar","email":"katkumar@in.ibm.com","username":"katarimanojkumar"},"change_message_id":"ea5237852c3cc3f37be2b9994f60e61e4da8058c","unresolved":false,"context_lines":[{"line_number":5989,"context_line":"            # is_dr_pool flag."},{"line_number":5990,"context_line":"            if pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027yes\u0027:"},{"line_number":5991,"context_line":"                is_dr_pool \u003d True"},{"line_number":5992,"context_line":"            elif pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027no\u0027:"},{"line_number":5993,"context_line":"                is_dr_pool \u003d False"},{"line_number":5994,"context_line":""},{"line_number":5995,"context_line":"            pool_stats \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"70c69962_dce5d47a","line":5992,"in_reply_to":"09df4553_77247db1","updated":"2021-08-03 17:21:07.000000000","message":"Thanks for the comment, will update in the next patchset.","commit_id":"ff402df67948eee78c48e1f221540fa491b42f0f"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"e563688022c0def49c2e745b290c957275987730","unresolved":true,"context_lines":[{"line_number":5998,"context_line":""},{"line_number":5999,"context_line":"            # Get the data_reduction information for pool and set"},{"line_number":6000,"context_line":"            # is_dr_pool flag."},{"line_number":6001,"context_line":"            is_dr_pool \u003d pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027yes\u0027"},{"line_number":6002,"context_line":""},{"line_number":6003,"context_line":"            pool_stats \u003d {"},{"line_number":6004,"context_line":"                \u0027pool_name\u0027: pool_data[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"55950f7f_878c5bbd","line":6001,"updated":"2021-08-10 18:09:05.000000000","message":"This change fixes a latent bug introduced in [1], and the fix should be backported to the same stable branches. Notice that [1] used lower case \u0027yes\u0027 on L852.\n\n[1] https://review.opendev.org/#/q/I00f1bd46bc8715591016f59c32651fc2c9cddf35","commit_id":"942cd7b733d19172d0e53e5cfb460fb156b93456"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ad073777dd222b17127b68e125c6f83429be363a","unresolved":true,"context_lines":[{"line_number":6025,"context_line":""},{"line_number":6026,"context_line":"            # Get the data_reduction information for pool and set"},{"line_number":6027,"context_line":"            # is_dr_pool flag."},{"line_number":6028,"context_line":"            is_dr_pool \u003d pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027yes\u0027"},{"line_number":6029,"context_line":""},{"line_number":6030,"context_line":"            pool_stats \u003d {"},{"line_number":6031,"context_line":"                \u0027pool_name\u0027: pool_data[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"1879c97d_4eae40a1","line":6028,"range":{"start_line":6028,"start_character":25,"end_line":6028,"end_character":56},"updated":"2021-08-16 15:47:14.000000000","message":"Is \u0027data_reduction\u0027 ever missing from pool_data?\n\nIn the previous code, that would result in pool_stats[\u0027data_reduction\u0027] \u003d None, but in this version it would cause an AttributeError on the lower() call.","commit_id":"08a695e02a70b347ef32a0c8fff1a92825896aff"},{"author":{"_account_id":32036,"name":"katari manoj kumar","email":"katkumar@in.ibm.com","username":"katarimanojkumar"},"change_message_id":"2ba282b62d99d6d336151d6ffd8030d4e1bbc69d","unresolved":true,"context_lines":[{"line_number":6025,"context_line":""},{"line_number":6026,"context_line":"            # Get the data_reduction information for pool and set"},{"line_number":6027,"context_line":"            # is_dr_pool flag."},{"line_number":6028,"context_line":"            is_dr_pool \u003d pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027yes\u0027"},{"line_number":6029,"context_line":""},{"line_number":6030,"context_line":"            pool_stats \u003d {"},{"line_number":6031,"context_line":"                \u0027pool_name\u0027: pool_data[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"4b491cf7_bf8e43b2","line":6028,"range":{"start_line":6028,"start_character":25,"end_line":6028,"end_character":56},"in_reply_to":"1879c97d_4eae40a1","updated":"2021-08-16 16:30:33.000000000","message":"Thanks for the review and pointing out the issue.\nWe need the conversion here as data_reduction can have any of the values \u0027yes\u0027 or \u0027Yes\u0027 or \u0027YES\u0027 from storage.\nTo address the issue you mentioned, i have added the None check before the line in the next patch.","commit_id":"08a695e02a70b347ef32a0c8fff1a92825896aff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"104ebc9dc81bfdfd1857cc4267510a6d1f5ef276","unresolved":true,"context_lines":[{"line_number":6026,"context_line":"            # Get the data_reduction information for pool and set"},{"line_number":6027,"context_line":"            # is_dr_pool flag."},{"line_number":6028,"context_line":"            if pool_data.get(\u0027data_reduction\u0027):"},{"line_number":6029,"context_line":"                is_dr_pool \u003d pool_data.get(\u0027data_reduction\u0027).lower() \u003d\u003d \u0027yes\u0027"},{"line_number":6030,"context_line":""},{"line_number":6031,"context_line":"            pool_stats \u003d {"},{"line_number":6032,"context_line":"                \u0027pool_name\u0027: pool_data[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":6,"id":"c97193cf_4bc90e57","line":6029,"range":{"start_line":6029,"start_character":16,"end_line":6029,"end_character":26},"updated":"2021-08-17 18:22:45.000000000","message":"initialized at line 6001","commit_id":"70a07ee7a2fda6c129903a21431fa2d499f4160d"}]}
