)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"903da1a4ea7681e9ffcba1b0c0f3bd166b4eb571","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Chaynika Saikia \u003ccsaikia@asu.edu\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-07-12 16:09:47 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"cinder backup is_incremental flag incorrect"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If --incremental flag is used and an icnremental backup"},{"line_number":10,"context_line":"fails, then before going to _full_backup(), it should"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1f1a1f67_9d80d58d","line":7,"range":{"start_line":7,"start_character":14,"end_line":7,"end_character":28},"updated":"2017-07-13 12:20:04.000000000","message":"-1: There is no such flag, we are correcting parent_id","commit_id":"29c80ca8b192bd3e3492de5fd44982a4cfaf24de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"903da1a4ea7681e9ffcba1b0c0f3bd166b4eb571","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"cinder backup is_incremental flag incorrect"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If --incremental flag is used and an icnremental backup"},{"line_number":10,"context_line":"fails, then before going to _full_backup(), it should"},{"line_number":11,"context_line":"update the parent_id of the backup to None."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1f1a1f67_3dcc2134","line":9,"range":{"start_line":9,"start_character":37,"end_line":9,"end_character":48},"updated":"2017-07-13 12:20:04.000000000","message":"nit: incremental","commit_id":"29c80ca8b192bd3e3492de5fd44982a4cfaf24de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"903da1a4ea7681e9ffcba1b0c0f3bd166b4eb571","unresolved":false,"context_lines":[{"line_number":9,"context_line":"If --incremental flag is used and an icnremental backup"},{"line_number":10,"context_line":"fails, then before going to _full_backup(), it should"},{"line_number":11,"context_line":"update the parent_id of the backup to None."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I22deff68d9376c2ecc88bd2d9d0e1b445d793123"},{"line_number":14,"context_line":"Closes-Bug: #1703013"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1f1a1f67_7d4cd9c6","line":12,"updated":"2017-07-13 12:20:04.000000000","message":"-1: That\u0027s not the only case, it should also update the parent_id field when you didn\u0027t request an incremental backup  but the driver is actually doing the incremental backup.","commit_id":"29c80ca8b192bd3e3492de5fd44982a4cfaf24de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":7,"context_line":"cinder backup sets incorrect parent_id"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If --incremental flag is used and an incremental backup"},{"line_number":10,"context_line":"fails, then before going to _full_backup(), it should"},{"line_number":11,"context_line":"update the parent_id of the backup to None."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Also for ceph driver, any backup after the first one"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1f1a1f67_bbeb70da","line":10,"range":{"start_line":10,"start_character":7,"end_line":10,"end_character":42},"updated":"2017-07-14 12:08:15.000000000","message":"-1: This is no longer true with the current code","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":15,"context_line":"is_incremental field for cinder backup-show \u003cbackup-id\u003e"},{"line_number":16,"context_line":"is misleading since it is set to False currently."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"The changes correctly updates the parent_id for a"},{"line_number":19,"context_line":"backup in case the incremental backup is successful."},{"line_number":20,"context_line":"If not, then it retains the parent_id as None. This"},{"line_number":21,"context_line":"will also update the parent_id to None if incremental"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1f1a1f67_bbc49060","line":18,"range":{"start_line":18,"start_character":0,"end_line":18,"end_character":11},"updated":"2017-07-14 12:08:15.000000000","message":"nit: \"This change\", or \"This patch\"","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"}],"cinder/backup/drivers/ceph.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"903da1a4ea7681e9ffcba1b0c0f3bd166b4eb571","unresolved":false,"context_lines":[{"line_number":685,"context_line":"            before \u003d time.time()"},{"line_number":686,"context_line":"            if backup.parent_id is None and not image_created:"},{"line_number":687,"context_line":"                backups \u003d objects.BackupList.get_all_by_volume("},{"line_number":688,"context_line":"                    self.context.elevated(), backup.volume_id)"},{"line_number":689,"context_line":"                latest_backup \u003d max(backups.objects,"},{"line_number":690,"context_line":"                                    key\u003dlambda x: x[\u0027data_timestamp\u0027] and"},{"line_number":691,"context_line":"                                    backup[\u0027created_at\u0027] \u003e"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_5df0fd67","line":688,"updated":"2017-07-13 12:20:04.000000000","message":"-1: Drivers should not query the Cinder DB.","commit_id":"29c80ca8b192bd3e3492de5fd44982a4cfaf24de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"903da1a4ea7681e9ffcba1b0c0f3bd166b4eb571","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                                    key\u003dlambda x: x[\u0027data_timestamp\u0027] and"},{"line_number":691,"context_line":"                                    backup[\u0027created_at\u0027] \u003e"},{"line_number":692,"context_line":"                                    x[\u0027data_timestamp\u0027])"},{"line_number":693,"context_line":"                backup.parent_id \u003d latest_backup.id"},{"line_number":694,"context_line":"            self._rbd_diff_transfer(volume_name, rbd_pool, base_name,"},{"line_number":695,"context_line":"                                    backup.container,"},{"line_number":696,"context_line":"                                    src_user\u003drbd_user,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_3dc301f0","line":693,"updated":"2017-07-13 12:20:04.000000000","message":"-1: This is a convoluted and unacceptable way of updating the parent_id, because it\u0027s relying on the fact that this change will be saved when the backup manager updates the backup\u0027s status and saves modified fields from the OVO.\n\nYou need to modify the backup method in the backup manager to accept backup method in drivers returning updates and have the ceph driver return the changed parent_id once it has finished.","commit_id":"29c80ca8b192bd3e3492de5fd44982a4cfaf24de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":714,"context_line":"                          {\u0027snapshot\u0027: new_snap, \u0027volume\u0027: volume_id})"},{"line_number":715,"context_line":"                source_rbd_image.remove_snap(new_snap)"},{"line_number":716,"context_line":"        if from_snap:"},{"line_number":717,"context_line":"            parent_id \u003d from_snap.split(\u0027.\u0027)"},{"line_number":718,"context_line":"            updates \u003d {\u0027parent_id\u0027: parent_id[1]}"},{"line_number":719,"context_line":"        return updates"},{"line_number":720,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_3b44001b","line":717,"updated":"2017-07-14 12:08:15.000000000","message":"nit: maybe a short comment to explain why you are doing this:\n\n # Naming format is backup.BACKUP-ID.snap.TIMESTAMP","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":928,"context_line":"        # If updates is empty then set parent_id to None. This will"},{"line_number":929,"context_line":"        # take care if --incremental flag is used in CLI but a full"},{"line_number":930,"context_line":"        # backup is performed instead"},{"line_number":931,"context_line":"        if not updates:"},{"line_number":932,"context_line":"            updates \u003d {\u0027parent_id\u0027: None}"},{"line_number":933,"context_line":"        return updates"},{"line_number":934,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_7b871863","line":931,"updated":"2017-07-14 12:08:15.000000000","message":"-1: Only change it to None if it\u0027s set\n\n if not updates and backup.parent_id:","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d3620abc05278bc3b84484243655fecee5f57ddb","unresolved":false,"context_lines":[{"line_number":715,"context_line":"                source_rbd_image.remove_snap(new_snap)"},{"line_number":716,"context_line":"        # We update the parent_id here. The parent_id is of the format:"},{"line_number":717,"context_line":"        # backup.BACKUP_ID.snap.TIMESTAMP. So we need to extract the"},{"line_number":718,"context_line":"        # backup_id only from from_snap"},{"line_number":719,"context_line":"        if from_snap:"},{"line_number":720,"context_line":"            parent_id \u003d from_snap.split(\u0027.\u0027)"},{"line_number":721,"context_line":"            updates \u003d {\u0027parent_id\u0027: parent_id[1]}"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_755256e0","line":718,"updated":"2017-07-17 11:30:09.000000000","message":"-1: parent_id does not have that format, it\u0027s an UUID, what\u0027s in that format is the snapshot name.\n\n # If we are doing incremental backup we need to update the parent_id\n # field to reflect it.  We can obtain the parent\u0027s id from the\n # snapshot name which has the format: backup.BACKUP_ID.snap.TIMESTAMP","commit_id":"6a2153062c294eba991981a91ff6095e8c4a4107"}],"cinder/backup/manager.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                        {\u0027status\u0027: previous_status,"},{"line_number":331,"context_line":"                         \u0027previous_status\u0027: \u0027error_backing-up\u0027})"},{"line_number":332,"context_line":"                self._update_backup_error(backup, six.text_type(err))"},{"line_number":333,"context_line":"        if updates is None:"},{"line_number":334,"context_line":"            updates \u003d {}"},{"line_number":335,"context_line":"        backup.update(updates)"},{"line_number":336,"context_line":"        backup.save()"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"        # Restore the original status."},{"line_number":339,"context_line":"        if snapshot_id:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_e059395a","line":336,"range":{"start_line":333,"start_character":0,"end_line":336,"end_character":21},"updated":"2017-07-14 12:08:15.000000000","message":"-1: If we know there are no updates we should not even bother updating the backup and then calling save.  In any case, this should go directly on L346 before the status update so we only do 1 update:\n\n if updates:\n     backup.update(updates)","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def _run_backup(self, context, backup, volume):"},{"line_number":361,"context_line":"        backup_service \u003d self.service.get_backup_driver(context)"},{"line_number":362,"context_line":"        updates_on_backup \u003d {}"},{"line_number":363,"context_line":"        properties \u003d utils.brick_get_connector_properties()"},{"line_number":364,"context_line":"        try:"},{"line_number":365,"context_line":"            backup_device \u003d self.volume_rpcapi.get_backup_device(context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_7b02780b","line":362,"updated":"2017-07-14 12:08:15.000000000","message":"nit: Not really necessary, all code paths will set the variable and this could mask a case were we don\u0027t.","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":375,"context_line":"                        not os.path.isdir(device_path)):"},{"line_number":376,"context_line":"                    if backup_device.secure_enabled:"},{"line_number":377,"context_line":"                        with open(device_path) as device_file:"},{"line_number":378,"context_line":"                            updates_on_backup \u003d backup_service.backup("},{"line_number":379,"context_line":"                                backup, device_file)"},{"line_number":380,"context_line":"                    else:"},{"line_number":381,"context_line":"                        with utils.temporary_chown(device_path):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_7b50380b","line":378,"updated":"2017-07-14 12:08:15.000000000","message":"nit: We could rename the variable to `updates` and would increase the readability of the code while conveying the same meaning as the long version counterpart.","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                    else:"},{"line_number":381,"context_line":"                        with utils.temporary_chown(device_path):"},{"line_number":382,"context_line":"                            with open(device_path) as device_file:"},{"line_number":383,"context_line":"                                updates_on_backup \u003d \\"},{"line_number":384,"context_line":"                                    backup_service.backup("},{"line_number":385,"context_line":"                                        backup, device_file)"},{"line_number":386,"context_line":"                # device_path is already file-like so no need to open it"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_1b3ca4c6","line":383,"range":{"start_line":383,"start_character":52,"end_line":383,"end_character":53},"updated":"2017-07-14 12:08:15.000000000","message":"-1: We don\u0027t use \\ to split lines, we should use parenthesis, the only exception is when we have multiple context managers, because they don\u0027t allow parenthesis.","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d3620abc05278bc3b84484243655fecee5f57ddb","unresolved":false,"context_lines":[{"line_number":332,"context_line":"                self._update_backup_error(backup, six.text_type(err))"},{"line_number":333,"context_line":"        if updates:"},{"line_number":334,"context_line":"            backup.update(updates)"},{"line_number":335,"context_line":"            backup.save()"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"        # Restore the original status."},{"line_number":338,"context_line":"        if snapshot_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_b5fb4e77","line":335,"updated":"2017-07-17 11:30:09.000000000","message":"-1: If we save here we\u0027ll be making 2 DB update queries instead of just 1 that we do if we move L333-L334 to L347:\n\n        backup.status \u003d fields.BackupStatus.AVAILABLE\n        backup.size \u003d volume[\u0027size\u0027]\n        if updates:\n            backup.update(updates)\n        backup.save()","commit_id":"6a2153062c294eba991981a91ff6095e8c4a4107"}],"cinder/tests/unit/backup/drivers/test_backup_ceph.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06fc141451b2e0d7430ffcd5dc168786514dcfdb","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                                                             \u0027conf_foo\u0027)"},{"line_number":472,"context_line":"                            rbdio \u003d linuxrbd.RBDVolumeIOWrapper(meta)"},{"line_number":473,"context_line":"                            output \u003d self.service.backup(self.backup, rbdio)"},{"line_number":474,"context_line":"                            is_dict \u003d False"},{"line_number":475,"context_line":"                            if type(output) is dict:"},{"line_number":476,"context_line":"                                is_dict \u003d True"},{"line_number":477,"context_line":"                            self.assertTrue(is_dict)"},{"line_number":478,"context_line":"                            self.assertEqual([\u0027popen_init\u0027,"},{"line_number":479,"context_line":"                                              \u0027read\u0027,"},{"line_number":480,"context_line":"                                              \u0027popen_init\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_9b9b14c8","line":477,"range":{"start_line":474,"start_character":0,"end_line":477,"end_character":52},"updated":"2017-07-14 12:08:15.000000000","message":"-1:\n\n self.assertIsInstance(output, dict)","commit_id":"a31d7823b633f30394f1a47343ebb8866e4efaa4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fe19222fba4cadeaa1c3314bd41bce8f8a50a99e","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                                                             \u0027conf_foo\u0027)"},{"line_number":472,"context_line":"                            rbdio \u003d linuxrbd.RBDVolumeIOWrapper(meta)"},{"line_number":473,"context_line":"                            output \u003d self.service.backup(self.backup, rbdio)"},{"line_number":474,"context_line":"                            self.assertIsInstance(output, dict)"},{"line_number":475,"context_line":"                            self.assertEqual([\u0027popen_init\u0027,"},{"line_number":476,"context_line":"                                              \u0027read\u0027,"},{"line_number":477,"context_line":"                                              \u0027popen_init\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_22a11d95","line":474,"updated":"2017-07-18 11:20:31.000000000","message":"nit: Should we check that the output has the parent_id key?","commit_id":"0599ea5c9691f535ae41af44724c539d82416fd6"},{"author":{"_account_id":26155,"name":"Chaynika Saikia","email":"csaikia@asu.edu","username":"csaikia"},"change_message_id":"2ce1b160540a1ae4764d4d4311879d9afa5bdbc3","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                                                             \u0027conf_foo\u0027)"},{"line_number":472,"context_line":"                            rbdio \u003d linuxrbd.RBDVolumeIOWrapper(meta)"},{"line_number":473,"context_line":"                            output \u003d self.service.backup(self.backup, rbdio)"},{"line_number":474,"context_line":"                            self.assertIsInstance(output, dict)"},{"line_number":475,"context_line":"                            self.assertEqual([\u0027popen_init\u0027,"},{"line_number":476,"context_line":"                                              \u0027read\u0027,"},{"line_number":477,"context_line":"                                              \u0027popen_init\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_1959defc","line":474,"in_reply_to":"1f1a1f67_22a11d95","updated":"2017-07-18 21:40:27.000000000","message":"In this test case, the parent_id will not exist in this output because this test case is not updating the parent_id in _backup_rbd() because from_snap variable is None. I will be adding new test cases where parent_id is set to None and a particular parent_id value","commit_id":"0599ea5c9691f535ae41af44724c539d82416fd6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fe19222fba4cadeaa1c3314bd41bce8f8a50a99e","unresolved":false,"context_lines":[{"line_number":472,"context_line":"                            rbdio \u003d linuxrbd.RBDVolumeIOWrapper(meta)"},{"line_number":473,"context_line":"                            output \u003d self.service.backup(self.backup, rbdio)"},{"line_number":474,"context_line":"                            self.assertIsInstance(output, dict)"},{"line_number":475,"context_line":"                            self.assertEqual([\u0027popen_init\u0027,"},{"line_number":476,"context_line":"                                              \u0027read\u0027,"},{"line_number":477,"context_line":"                                              \u0027popen_init\u0027,"},{"line_number":478,"context_line":"                                              \u0027write\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_e2bfa5a7","line":475,"updated":"2017-07-18 11:20:31.000000000","message":"I think we should also have a test that doesn\u0027t set the parent_id and another that sets the parent_id to None.","commit_id":"0599ea5c9691f535ae41af44724c539d82416fd6"},{"author":{"_account_id":26155,"name":"Chaynika Saikia","email":"csaikia@asu.edu","username":"csaikia"},"change_message_id":"2ce1b160540a1ae4764d4d4311879d9afa5bdbc3","unresolved":false,"context_lines":[{"line_number":472,"context_line":"                            rbdio \u003d linuxrbd.RBDVolumeIOWrapper(meta)"},{"line_number":473,"context_line":"                            output \u003d self.service.backup(self.backup, rbdio)"},{"line_number":474,"context_line":"                            self.assertIsInstance(output, dict)"},{"line_number":475,"context_line":"                            self.assertEqual([\u0027popen_init\u0027,"},{"line_number":476,"context_line":"                                              \u0027read\u0027,"},{"line_number":477,"context_line":"                                              \u0027popen_init\u0027,"},{"line_number":478,"context_line":"                                              \u0027write\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_d976c670","line":475,"in_reply_to":"1f1a1f67_e2bfa5a7","updated":"2017-07-18 21:40:27.000000000","message":"Thanks! Will do.","commit_id":"0599ea5c9691f535ae41af44724c539d82416fd6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1f994ecee7e4b7613c68ccf0ab7f62b01a7da2da","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                                                             \u0027conf_foo\u0027)"},{"line_number":472,"context_line":"                            rbdio \u003d linuxrbd.RBDVolumeIOWrapper(meta)"},{"line_number":473,"context_line":"                            output \u003d self.service.backup(self.backup, rbdio)"},{"line_number":474,"context_line":"                            self.assertIsInstance(output, dict)"},{"line_number":475,"context_line":"                            self.assertEqual([\u0027popen_init\u0027,"},{"line_number":476,"context_line":"                                              \u0027read\u0027,"},{"line_number":477,"context_line":"                                              \u0027popen_init\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f1a1f67_55bb31e4","line":474,"updated":"2017-07-21 13:04:03.000000000","message":"-1: If we know dict will not have parent_id we should check it appropriately:\n\n self.assertDictEqual({}, output)","commit_id":"ff855430d96450d412583198b6a4a4ee0a3f18b2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1f994ecee7e4b7613c68ccf0ab7f62b01a7da2da","unresolved":false,"context_lines":[{"line_number":489,"context_line":"    @common_mocks"},{"line_number":490,"context_line":"    @mock.patch(\u0027fcntl.fcntl\u0027, spec\u003dTrue)"},{"line_number":491,"context_line":"    @mock.patch(\u0027subprocess.Popen\u0027, spec\u003dTrue)"},{"line_number":492,"context_line":"    def test_backup_volume_from_rbd_set_parent_id(self, mock_popen,"},{"line_number":493,"context_line":"                                                  mock_fnctl):"},{"line_number":494,"context_line":"        backup_name \u003d self.service._get_backup_base_name("},{"line_number":495,"context_line":"            self.backup_id, diff_format\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f1a1f67_0b27da66","line":492,"updated":"2017-07-21 13:04:03.000000000","message":"-1: Tests scope is too broad, you are running backup method and testing backup and _backup_rbd methods.\n\nYou should test backup method mocking _backup_rbd, and then test _backup_rbd on its own.","commit_id":"ff855430d96450d412583198b6a4a4ee0a3f18b2"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":491,"context_line":"    @mock.patch(\u0027subprocess.Popen\u0027, spec\u003dTrue)"},{"line_number":492,"context_line":"    def test_backup_volume_from_rbd_set_parent_id(self, mock_popen,"},{"line_number":493,"context_line":"                                                  mock_fnctl):"},{"line_number":494,"context_line":"        with mock.patch.object(self.service, \u0027_backup_metadata\u0027):"},{"line_number":495,"context_line":"            with mock.patch.object(self.service, \u0027_backup_rbd\u0027) as \\"},{"line_number":496,"context_line":"                    mock_backup_rbd:"},{"line_number":497,"context_line":"                mock_backup_rbd.return_value \u003d {\u0027parent_id\u0027: \u0027mock\u0027}"},{"line_number":498,"context_line":"                image \u003d self.service.rbd.Image()"},{"line_number":499,"context_line":"                meta \u003d linuxrbd.RBDImageMetadata(image,"},{"line_number":500,"context_line":"                                                 \u0027pool_foo\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_5e658f44","line":497,"range":{"start_line":494,"start_character":0,"end_line":497,"end_character":3},"updated":"2017-07-25 12:37:33.000000000","message":"nit: We don\u0027t need to nest context managers like this:\n\n with mock.patch.object(...), \\\n         mock.patch.object(...) as \\\n\nAnd if you reverse the order of the mocks and you change mock_backup_rbd with mock_backup you\u0027ll be able to write it in a single line making it more readable:\n\n        with mock.patch.object(self.service, \u0027_backup_rbd\u0027) as mock_backup_rbd, \\\n            mock.patch.object(self.service, \u0027_backup_metadata\u0027):","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":502,"context_line":"                                                 \u0027conf_foo\u0027)"},{"line_number":503,"context_line":"                rbdio \u003d linuxrbd.RBDVolumeIOWrapper(meta)"},{"line_number":504,"context_line":"                output \u003d self.service.backup(self.backup, rbdio)"},{"line_number":505,"context_line":"                self.assertEqual(output[\u0027parent_id\u0027], \u0027mock\u0027)"},{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    @common_mocks"},{"line_number":508,"context_line":"    @mock.patch(\u0027fcntl.fcntl\u0027, spec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_9eee57b2","line":505,"updated":"2017-07-25 12:37:33.000000000","message":"-1: Asserts should be expected, actual:\n\n self.assertEqual(\u0027mock\u0027, output[\u0027parent_id\u0027])\n\nBut since you know it\u0027s a dict, you\u0027ll want to check it all:\n\n self.assertDictEqual({\u0027parent_id\u0027: \u0027mock\u0027}, output)","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":567,"context_line":"    def test_backup_rbd_set_parent_id(self, mock_popen, mock_fnctl):"},{"line_number":568,"context_line":"        backup_name \u003d self.service._get_backup_base_name("},{"line_number":569,"context_line":"            self.backup_id, diff_format\u003dTrue)"},{"line_number":570,"context_line":"        vol_name \u003d self.volume[\u0027name\u0027]"},{"line_number":571,"context_line":"        vol_length \u003d self.service._get_volume_size_gb(self.volume)"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"        def mock_write_data():"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_9e8d37d5","line":570,"updated":"2017-07-25 12:37:33.000000000","message":"nit: volume.name","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":568,"context_line":"        backup_name \u003d self.service._get_backup_base_name("},{"line_number":569,"context_line":"            self.backup_id, diff_format\u003dTrue)"},{"line_number":570,"context_line":"        vol_name \u003d self.volume[\u0027name\u0027]"},{"line_number":571,"context_line":"        vol_length \u003d self.service._get_volume_size_gb(self.volume)"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"        def mock_write_data():"},{"line_number":574,"context_line":"            self.volume_file.seek(0)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_5e66af2b","line":571,"updated":"2017-07-25 12:37:33.000000000","message":"-1: Why do we need this and it\u0027s not good enough with self.volume_size of self.volume.size?","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":571,"context_line":"        vol_length \u003d self.service._get_volume_size_gb(self.volume)"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"        def mock_write_data():"},{"line_number":574,"context_line":"            self.volume_file.seek(0)"},{"line_number":575,"context_line":"            data \u003d self.volume_file.read(self.data_length)"},{"line_number":576,"context_line":"            self.callstack.append(\u0027write\u0027)"},{"line_number":577,"context_line":"            checksum.update(data)"},{"line_number":578,"context_line":"            test_file.write(data)"},{"line_number":579,"context_line":""},{"line_number":580,"context_line":"        def mock_read_data():"},{"line_number":581,"context_line":"            self.callstack.append(\u0027read\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_01dd666f","line":578,"range":{"start_line":574,"start_character":0,"end_line":578,"end_character":33},"updated":"2017-07-25 12:37:33.000000000","message":"-1: Unless I\u0027m mistaken this is totally useless, we are checking that we can read and write a file that was setup by the setUp method.","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                               p1hook\u003dmock_read_data,"},{"line_number":587,"context_line":"                               p2hook\u003dmock_write_data)"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"        self.mock_rbd.RBD().list \u003d mock.Mock()"},{"line_number":590,"context_line":"        self.mock_rbd.RBD().list.return_value \u003d [backup_name]"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"        with mock.patch.object(self.service, \u0027_snap_exists\u0027):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_a168da86","line":589,"updated":"2017-07-25 12:37:33.000000000","message":"-1: Unnecessary, there\u0027s already a Mock there","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":589,"context_line":"        self.mock_rbd.RBD().list \u003d mock.Mock()"},{"line_number":590,"context_line":"        self.mock_rbd.RBD().list.return_value \u003d [backup_name]"},{"line_number":591,"context_line":""},{"line_number":592,"context_line":"        with mock.patch.object(self.service, \u0027_snap_exists\u0027):"},{"line_number":593,"context_line":"            with mock.patch.object(self.service, \u0027_get_backup_base_name\u0027) as \\"},{"line_number":594,"context_line":"                    mock_get_backup_base_name:"},{"line_number":595,"context_line":"                with mock.patch.object(self.service, \u0027_get_most_recent_snap\u0027) as \\"},{"line_number":596,"context_line":"                        mock_get_most_recent_snap:"},{"line_number":597,"context_line":"                    with tempfile.NamedTemporaryFile() as test_file:"},{"line_number":598,"context_line":"                        mock_get_backup_base_name.return_value \u003d backup_name"},{"line_number":599,"context_line":"                        mock_get_most_recent_snap.return_value \u003d ("},{"line_number":600,"context_line":"                            \u0027backup.mock.snap.153464362.12\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_9e541723","line":597,"range":{"start_line":592,"start_character":0,"end_line":597,"end_character":68},"updated":"2017-07-25 12:37:33.000000000","message":"-1: Too many nested contexts, better use this form:\n\n with X, \\\n         Y, \\\n         Z, \\\n         A as test_file:","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":598,"context_line":"                        mock_get_backup_base_name.return_value \u003d backup_name"},{"line_number":599,"context_line":"                        mock_get_most_recent_snap.return_value \u003d ("},{"line_number":600,"context_line":"                            \u0027backup.mock.snap.153464362.12\u0027)"},{"line_number":601,"context_line":"                        checksum \u003d hashlib.sha256()"},{"line_number":602,"context_line":"                        image \u003d self.service.rbd.Image()"},{"line_number":603,"context_line":"                        meta \u003d linuxrbd.RBDImageMetadata(image,"},{"line_number":604,"context_line":"                                                         \u0027pool_foo\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_814dd673","line":601,"updated":"2017-07-25 12:37:33.000000000","message":"-1: Since this is going to be used by the mock_write_data please define it above the method definition so we don\u0027t have to wonder if it\u0027s a global variable.  Although like I mention above, this whole checksum thing seems to be useless.","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":608,"context_line":"                        rbdio.seek(0)"},{"line_number":609,"context_line":"                        output \u003d self.service._backup_rbd(self.backup, rbdio,"},{"line_number":610,"context_line":"                                                          vol_name, vol_length)"},{"line_number":611,"context_line":"                        self.assertEqual(output[\u0027parent_id\u0027], \u0027mock\u0027)"},{"line_number":612,"context_line":"                        self.assertEqual([\u0027popen_init\u0027,"},{"line_number":613,"context_line":"                                          \u0027read\u0027,"},{"line_number":614,"context_line":"                                          \u0027popen_init\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_1e3ee7d9","line":611,"updated":"2017-07-25 12:37:33.000000000","message":"-1: expected, actual","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":617,"context_line":"                                          \u0027communicate\u0027], self.callstack)"},{"line_number":618,"context_line":"                        # Ensure the files are equal"},{"line_number":619,"context_line":"                        self.assertEqual(checksum.digest(),"},{"line_number":620,"context_line":"                                         self.checksum.digest())"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    @common_mocks"},{"line_number":623,"context_line":"    @mock.patch(\u0027fcntl.fcntl\u0027, spec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_4157ee03","line":620,"updated":"2017-07-25 12:37:33.000000000","message":"-1: expected, actual\n\nAnd I also believe this is testing nothing... Just that the mock_write_data method has been called.","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"}],"cinder/tests/unit/backup/test_backup.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":649,"context_line":"        with mock.patch.object(self.backup_mgr.service, \u0027get_backup_driver\u0027) as \\"},{"line_number":650,"context_line":"                mock_get_backup_driver:"},{"line_number":651,"context_line":"            mock_get_backup_driver.return_value \u003d ("},{"line_number":652,"context_line":"                self.backup_mgr.service.get_backup_driver(self.ctxt))"},{"line_number":653,"context_line":"            backup_service \u003d mock_get_backup_driver.return_value"},{"line_number":654,"context_line":"            with mock.patch.object(self.backup_mgr, \u0027_detach_device\u0027):"},{"line_number":655,"context_line":"                with mock.patch.object(backup_service, \u0027backup\u0027) as \\"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_c1487ecd","line":652,"updated":"2017-07-25 12:37:33.000000000","message":"-1: This is pointless, you are setting it to the same value, a Mock","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":653,"context_line":"            backup_service \u003d mock_get_backup_driver.return_value"},{"line_number":654,"context_line":"            with mock.patch.object(self.backup_mgr, \u0027_detach_device\u0027):"},{"line_number":655,"context_line":"                with mock.patch.object(backup_service, \u0027backup\u0027) as \\"},{"line_number":656,"context_line":"                        mock_backup:"},{"line_number":657,"context_line":"                    mock_backup.return_value \u003d ("},{"line_number":658,"context_line":"                        {\u0027parent_id\u0027: None})"},{"line_number":659,"context_line":"                    device_path \u003d \u0027/fake/disk/path/\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_21118aad","line":656,"updated":"2017-07-25 12:37:33.000000000","message":"-1: backup_service is already a Mock, we don\u0027t need to mock it again","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":669,"context_line":"                    self.backup_mgr.create_backup(self.ctxt, backup)"},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        backup \u003d db.backup_get(self.ctxt, backup.id)"},{"line_number":672,"context_line":"        self.assertEqual(fields.BackupStatus.AVAILABLE, backup[\u0027status\u0027])"},{"line_number":673,"context_line":"        self.assertEqual(vol_size, backup[\u0027size\u0027])"},{"line_number":674,"context_line":"        self.assertIsNone(backup.parent_id)"},{"line_number":675,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_e11f026e","line":672,"updated":"2017-07-25 12:37:33.000000000","message":"nit: backup.status","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        backup \u003d db.backup_get(self.ctxt, backup.id)"},{"line_number":672,"context_line":"        self.assertEqual(fields.BackupStatus.AVAILABLE, backup[\u0027status\u0027])"},{"line_number":673,"context_line":"        self.assertEqual(vol_size, backup[\u0027size\u0027])"},{"line_number":674,"context_line":"        self.assertIsNone(backup.parent_id)"},{"line_number":675,"context_line":""},{"line_number":676,"context_line":"    @mock.patch(\u0027cinder.utils.brick_get_connector_properties\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_413b6eea","line":673,"updated":"2017-07-25 12:37:33.000000000","message":"nit: backup.size","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":678,"context_line":"    @mock.patch(\u0027cinder.utils.temporary_chown\u0027)"},{"line_number":679,"context_line":"    @mock.patch(\u0027six.moves.builtins.open\u0027)"},{"line_number":680,"context_line":"    @mock.patch.object(os.path, \u0027isdir\u0027, return_value\u003dTrue)"},{"line_number":681,"context_line":"    def test_create_backup_change_parent_id2(self, mock_isdir, mock_open,"},{"line_number":682,"context_line":"                                             mock_chown, mock_backup_device,"},{"line_number":683,"context_line":"                                             mock_brick):"},{"line_number":684,"context_line":"        vol_size \u003d 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_c1fd1e3b","line":681,"updated":"2017-07-25 12:37:33.000000000","message":"-1: The name does not give any info on what it does, and this and the previous case would be a good example for a ddt","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":684,"context_line":"        vol_size \u003d 1"},{"line_number":685,"context_line":"        vol_id \u003d self._create_volume_db_entry(size\u003dvol_size)"},{"line_number":686,"context_line":"        backup \u003d self._create_backup_db_entry(volume_id\u003dvol_id)"},{"line_number":687,"context_line":"        parent_backup \u003d self._create_backup_db_entry(size\u003dvol_size)"},{"line_number":688,"context_line":""},{"line_number":689,"context_line":"        with mock.patch.object(self.backup_mgr.service, \u0027get_backup_driver\u0027) as \\"},{"line_number":690,"context_line":"                mock_get_backup_driver:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_c1807e8e","line":687,"updated":"2017-07-25 12:37:33.000000000","message":"nit: do we really need to create a parent backup? or is it enough with having a valid UUID for that backup?","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        with mock.patch.object(self.backup_mgr.service, \u0027get_backup_driver\u0027) as \\"},{"line_number":690,"context_line":"                mock_get_backup_driver:"},{"line_number":691,"context_line":"            mock_get_backup_driver.return_value \u003d ("},{"line_number":692,"context_line":"                self.backup_mgr.service.get_backup_driver(self.ctxt))"},{"line_number":693,"context_line":"            backup_service \u003d mock_get_backup_driver.return_value"},{"line_number":694,"context_line":"            with mock.patch.object(self.backup_mgr, \u0027_detach_device\u0027):"},{"line_number":695,"context_line":"                with mock.patch.object(backup_service, \u0027backup\u0027) as \\"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_61655245","line":692,"updated":"2017-07-25 12:37:33.000000000","message":"-1: Same as above","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":693,"context_line":"            backup_service \u003d mock_get_backup_driver.return_value"},{"line_number":694,"context_line":"            with mock.patch.object(self.backup_mgr, \u0027_detach_device\u0027):"},{"line_number":695,"context_line":"                with mock.patch.object(backup_service, \u0027backup\u0027) as \\"},{"line_number":696,"context_line":"                        mock_backup:"},{"line_number":697,"context_line":"                    mock_backup.return_value \u003d ("},{"line_number":698,"context_line":"                        {\u0027parent_id\u0027: parent_backup[\u0027id\u0027]})"},{"line_number":699,"context_line":"                    device_path \u003d \u0027/fake/disk/path/\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_61caf21a","line":696,"updated":"2017-07-25 12:37:33.000000000","message":"-1: same as above","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":695,"context_line":"                with mock.patch.object(backup_service, \u0027backup\u0027) as \\"},{"line_number":696,"context_line":"                        mock_backup:"},{"line_number":697,"context_line":"                    mock_backup.return_value \u003d ("},{"line_number":698,"context_line":"                        {\u0027parent_id\u0027: parent_backup[\u0027id\u0027]})"},{"line_number":699,"context_line":"                    device_path \u003d \u0027/fake/disk/path/\u0027"},{"line_number":700,"context_line":"                    attach_info \u003d {\u0027device\u0027: {\u0027path\u0027: device_path}}"},{"line_number":701,"context_line":"                    mock_attach_device \u003d self.mock_object(self.backup_mgr,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_8131560e","line":698,"updated":"2017-07-25 12:37:33.000000000","message":"nit: backup.id","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":710,"context_line":""},{"line_number":711,"context_line":"        backup \u003d db.backup_get(self.ctxt, backup.id)"},{"line_number":712,"context_line":"        self.assertEqual(fields.BackupStatus.AVAILABLE, backup[\u0027status\u0027])"},{"line_number":713,"context_line":"        self.assertEqual(vol_size, backup[\u0027size\u0027])"},{"line_number":714,"context_line":"        self.assertEqual(parent_backup.id, backup.parent_id)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    @mock.patch(\u0027cinder.utils.brick_get_connector_properties\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_a1545a22","line":713,"updated":"2017-07-25 12:37:33.000000000","message":"nit: backup.size\n\nWe should be consistent in our notation, and we are using attribute notation almost everywhere else.","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":727,"context_line":"        with mock.patch.object(self.backup_mgr.service, \u0027get_backup_driver\u0027) as \\"},{"line_number":728,"context_line":"                mock_get_backup_driver:"},{"line_number":729,"context_line":"            mock_get_backup_driver.return_value \u003d ("},{"line_number":730,"context_line":"                self.backup_mgr.service.get_backup_driver(self.ctxt))"},{"line_number":731,"context_line":"            backup_service \u003d mock_get_backup_driver.return_value"},{"line_number":732,"context_line":"            with mock.patch.object(self.backup_mgr, \u0027_detach_device\u0027):"},{"line_number":733,"context_line":"                with mock.patch.object(backup_service, \u0027backup\u0027) as \\"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_2c09812b","line":730,"updated":"2017-07-25 12:37:33.000000000","message":"-1: same as above","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":731,"context_line":"            backup_service \u003d mock_get_backup_driver.return_value"},{"line_number":732,"context_line":"            with mock.patch.object(self.backup_mgr, \u0027_detach_device\u0027):"},{"line_number":733,"context_line":"                with mock.patch.object(backup_service, \u0027backup\u0027) as \\"},{"line_number":734,"context_line":"                        mock_backup:"},{"line_number":735,"context_line":"                    mock_backup.side_effect \u003d FakeBackupException(\u0027fake\u0027)"},{"line_number":736,"context_line":"                    device_path \u003d \u0027/fake/disk/path/\u0027"},{"line_number":737,"context_line":"                    attach_info \u003d {\u0027device\u0027: {\u0027path\u0027: device_path}}"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_cc0d5536","line":734,"updated":"2017-07-25 12:37:33.000000000","message":"-1: same as above","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":749,"context_line":"                                      self.ctxt, backup)"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        vol \u003d db.volume_get(self.ctxt, vol_id)"},{"line_number":752,"context_line":"        self.assertEqual(\u0027available\u0027, vol[\u0027status\u0027])"},{"line_number":753,"context_line":"        self.assertEqual(\u0027error_backing-up\u0027, vol[\u0027previous_status\u0027])"},{"line_number":754,"context_line":"        backup \u003d db.backup_get(self.ctxt, backup.id)"},{"line_number":755,"context_line":"        self.assertEqual(fields.BackupStatus.ERROR, backup[\u0027status\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_6c28a985","line":752,"updated":"2017-07-25 12:37:33.000000000","message":"nit: vol.status","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        vol \u003d db.volume_get(self.ctxt, vol_id)"},{"line_number":752,"context_line":"        self.assertEqual(\u0027available\u0027, vol[\u0027status\u0027])"},{"line_number":753,"context_line":"        self.assertEqual(\u0027error_backing-up\u0027, vol[\u0027previous_status\u0027])"},{"line_number":754,"context_line":"        backup \u003d db.backup_get(self.ctxt, backup.id)"},{"line_number":755,"context_line":"        self.assertEqual(fields.BackupStatus.ERROR, backup[\u0027status\u0027])"},{"line_number":756,"context_line":"        self.assertTrue(mock_backup.called)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_4c2ba581","line":753,"updated":"2017-07-25 12:37:33.000000000","message":"nit: vol.previous_status","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":752,"context_line":"        self.assertEqual(\u0027available\u0027, vol[\u0027status\u0027])"},{"line_number":753,"context_line":"        self.assertEqual(\u0027error_backing-up\u0027, vol[\u0027previous_status\u0027])"},{"line_number":754,"context_line":"        backup \u003d db.backup_get(self.ctxt, backup.id)"},{"line_number":755,"context_line":"        self.assertEqual(fields.BackupStatus.ERROR, backup[\u0027status\u0027])"},{"line_number":756,"context_line":"        self.assertTrue(mock_backup.called)"},{"line_number":757,"context_line":""},{"line_number":758,"context_line":"    @mock.patch(\u0027cinder.utils.brick_get_connector_properties\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_0c259dab","line":755,"updated":"2017-07-25 12:37:33.000000000","message":"nit: backup.status","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":760,"context_line":"    @mock.patch(\u0027cinder.utils.temporary_chown\u0027)"},{"line_number":761,"context_line":"    @mock.patch(\u0027six.moves.builtins.open\u0027)"},{"line_number":762,"context_line":"    @mock.patch.object(os.path, \u0027isdir\u0027, return_value\u003dTrue)"},{"line_number":763,"context_line":"    @ddt.data({\u0027parent_id\u0027: \u0027abc\u0027}, {\u0027parent_id\u0027: None}, None)"},{"line_number":764,"context_line":"    def test_run_backup_with_dir_device_path(self, backup_output, mock_isdir,"},{"line_number":765,"context_line":"                                             mock_open,"},{"line_number":766,"context_line":"                                             mock_chown,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_61e0b2cd","line":763,"updated":"2017-07-25 12:37:33.000000000","message":"-1: In this case it\u0027s irrelevant the data we return, since the method is just going to pass it along to the caller.  Instead of running 3 tests we can just make backup return `mock.sentinel.backup_update` and check against that same object afterwards.","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":768,"context_line":"                                             mock_brick):"},{"line_number":769,"context_line":"        backup_service \u003d lambda: None"},{"line_number":770,"context_line":"        backup_service.backup \u003d mock.Mock()"},{"line_number":771,"context_line":"        backup_service.backup.return_value \u003d backup_output"},{"line_number":772,"context_line":"        self.backup_mgr.service.get_backup_driver \u003d lambda x: backup_service"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        vol_id \u003d self._create_volume_db_entry()"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_a1721a48","line":771,"updated":"2017-07-25 12:37:33.000000000","message":"-1: add it to the previous line:\n\n    backup_service.backup \u003d mock.Mock(return_value\u003dbackup_output)","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a4fa0a14521276d071165771a53c0049f8935c55","unresolved":false,"context_lines":[{"line_number":787,"context_line":"        mock_open.assert_not_called()"},{"line_number":788,"context_line":"        backup_service.backup.assert_called_once_with("},{"line_number":789,"context_line":"            backup, device_path)"},{"line_number":790,"context_line":"        self.assertEqual(output, backup_output)"},{"line_number":791,"context_line":""},{"line_number":792,"context_line":"    @mock.patch(\u0027cinder.backup.manager.BackupManager._run_backup\u0027)"},{"line_number":793,"context_line":"    @ddt.data((fields.SnapshotStatus.BACKING_UP, \u0027available\u0027),"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_ec401938","line":790,"updated":"2017-07-25 12:37:33.000000000","message":"-1: expected, actual","commit_id":"14290dc8354de77127b8fec98f13d1e17980b540"}]}