)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7c47f120189372cf967a1513bb6df03a8656d0ac","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When importing backup metadata only required fields were being imported,"},{"line_number":10,"context_line":"and all other fields were being ignored."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This lead to some serious problems when importing metadata back, since"},{"line_number":13,"context_line":"you would lose not only the original Volume ID, but also relevant parent"},{"line_number":14,"context_line":"id information needed to restore incremental backups."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a016987_a17b3abd","line":12,"updated":"2015-05-22 13:54:11.000000000","message":"lead \u003d\u003e leads","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"34479edf4e7723197afe2ba400e851437843ee2a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When importing backup metadata only required fields were being imported,"},{"line_number":10,"context_line":"and all other fields were being ignored."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This lead to some serious problems when importing metadata back, since"},{"line_number":13,"context_line":"you would lose not only the original Volume ID, but also relevant parent"},{"line_number":14,"context_line":"id information needed to restore incremental backups."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7a016987_950c35ed","line":12,"updated":"2015-05-29 09:26:03.000000000","message":"leads (present tense) or led (past tense).  Irregular verbs are a pain :-)","commit_id":"29ab16591cd785326009cf2e06f0a7f9166dd34f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7d339b87dcc2ff572333e2e7146066c2752f5fa2","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When importing backup metadata only required fields were being imported,"},{"line_number":10,"context_line":"and all other fields were being ignored."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This lead to some serious problems when importing metadata back, since"},{"line_number":13,"context_line":"you would lose not only the original Volume ID, but also relevant parent"},{"line_number":14,"context_line":"id information needed to restore incremental backups."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7a016987_7d94ff89","line":12,"in_reply_to":"7a016987_950c35ed","updated":"2015-05-29 10:55:04.000000000","message":"Ooops, my bad.","commit_id":"29ab16591cd785326009cf2e06f0a7f9166dd34f"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"38ac296c0a41757c7c44f3c63db12c50b1b4940f","unresolved":false,"context_lines":[{"line_number":17,"context_line":"records with the right ID."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: #1455043"},{"line_number":20,"context_line":"Closes-Bug: #1476416"},{"line_number":21,"context_line":"Change-Id: Ia42ba73e9078d993c63f8e16308151ad11721ea9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"3a50d1a3_e841fc0a","line":20,"updated":"2015-07-29 15:32:03.000000000","message":"It\u0027s only related bug. Please, take a look on my comments\nhttps://bugs.launchpad.net/cinder/+bug/1476416","commit_id":"54011bb46636d07b8cf5fba1506d0d5261af322a"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3420bd400755d9999c1ce9158a5de62a43a9efa2","unresolved":false,"context_lines":[{"line_number":17,"context_line":"records with the right ID."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: #1455043"},{"line_number":20,"context_line":"Closes-Bug: #1476416"},{"line_number":21,"context_line":"Change-Id: Ia42ba73e9078d993c63f8e16308151ad11721ea9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"3a50d1a3_2eb9fe79","line":20,"in_reply_to":"3a50d1a3_e809d185","updated":"2015-07-30 12:17:29.000000000","message":"No problem, but you scared me there.  ;-)","commit_id":"54011bb46636d07b8cf5fba1506d0d5261af322a"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"80c80ae96d4e6ba35fede9ed9c9ee8c6c91686db","unresolved":false,"context_lines":[{"line_number":17,"context_line":"records with the right ID."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: #1455043"},{"line_number":20,"context_line":"Closes-Bug: #1476416"},{"line_number":21,"context_line":"Change-Id: Ia42ba73e9078d993c63f8e16308151ad11721ea9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"3a50d1a3_e809d185","line":20,"in_reply_to":"3a50d1a3_e841fc0a","updated":"2015-07-30 10:28:48.000000000","message":"Gorka, I\u0027m very sorry. I\u0027ve tested it in a wrong way:(","commit_id":"54011bb46636d07b8cf5fba1506d0d5261af322a"}],"cinder/backup/manager.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e3eba4fbf427fc0da215abfa30a531fef9b46a24","unresolved":false,"context_lines":[{"line_number":597,"context_line":"            missing_opts \u003d required_import_options - set(backup_options)"},{"line_number":598,"context_line":"            if missing_opts:"},{"line_number":599,"context_line":"                msg \u003d (_(\u0027Backup metadata received from driver for \u0027"},{"line_number":600,"context_line":"                         \u0027import is missing fields:  %s.\u0027) %"},{"line_number":601,"context_line":"                       \u0027, \u0027.join(missing_opts))"},{"line_number":602,"context_line":"                self.db.backup_update(context,"},{"line_number":603,"context_line":"                                      backup_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_b53667e5","line":600,"updated":"2015-05-15 15:07:34.000000000","message":"I know this message was already here, but it seems pretty confusing.  What should an admin/user make of \"metadata received from driver\" as far as what this actually means or what to do about it?  I don\u0027t really get what this means myself.\n\n(Maybe don\u0027t worry about this for now, but something to think about.)","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aa117b8d67e9ebbc6343945c4aa1f59aae706da8","unresolved":false,"context_lines":[{"line_number":597,"context_line":"            missing_opts \u003d required_import_options - set(backup_options)"},{"line_number":598,"context_line":"            if missing_opts:"},{"line_number":599,"context_line":"                msg \u003d (_(\u0027Backup metadata received from driver for \u0027"},{"line_number":600,"context_line":"                         \u0027import is missing fields:  %s.\u0027) %"},{"line_number":601,"context_line":"                       \u0027, \u0027.join(missing_opts))"},{"line_number":602,"context_line":"                self.db.backup_update(context,"},{"line_number":603,"context_line":"                                      backup_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_bf9f5d02","line":600,"in_reply_to":"7a016987_b53667e5","updated":"2015-05-15 19:12:41.000000000","message":"I could change the message to something different, like: \"Driver successfully decoded imported backup data, but there\u0027s missing fields (%s). Either data is corrupt or driver has a bug\"","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e3eba4fbf427fc0da215abfa30a531fef9b46a24","unresolved":false,"context_lines":[{"line_number":605,"context_line":"                                       \u0027fail_reason\u0027: msg})"},{"line_number":606,"context_line":"                raise exception.InvalidBackup(reason\u003dmsg)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"            # Overwwrite some fields"},{"line_number":609,"context_line":"            backup_options[\u0027status\u0027] \u003d \u0027available\u0027"},{"line_number":610,"context_line":"            backup_options[\u0027service\u0027] \u003d self.driver_name"},{"line_number":611,"context_line":"            backup_options[\u0027availability_zone\u0027] \u003d self.az"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_23c000e0","line":608,"updated":"2015-05-15 15:07:34.000000000","message":"\"Overwrite\" typo","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aa117b8d67e9ebbc6343945c4aa1f59aae706da8","unresolved":false,"context_lines":[{"line_number":605,"context_line":"                                       \u0027fail_reason\u0027: msg})"},{"line_number":606,"context_line":"                raise exception.InvalidBackup(reason\u003dmsg)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"            # Overwwrite some fields"},{"line_number":609,"context_line":"            backup_options[\u0027status\u0027] \u003d \u0027available\u0027"},{"line_number":610,"context_line":"            backup_options[\u0027service\u0027] \u003d self.driver_name"},{"line_number":611,"context_line":"            backup_options[\u0027availability_zone\u0027] \u003d self.az"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_1f7d51b1","line":608,"in_reply_to":"7a016987_23c000e0","updated":"2015-05-15 19:12:41.000000000","message":"Thanks, I\u0027ll wait for more mistakes/changes to come up to fix this.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7c47f120189372cf967a1513bb6df03a8656d0ac","unresolved":false,"context_lines":[{"line_number":623,"context_line":""},{"line_number":624,"context_line":"            else:"},{"line_number":625,"context_line":"                if backup.status not in (\u0027available\u0027, \u0027error\u0027, \u0027deleted\u0027):"},{"line_number":626,"context_line":"                    msg \u003d (_(\"Backup %(id)s exist in DB with status %(status)s\""},{"line_number":627,"context_line":"                             \" so we can\u0027t update it\") %"},{"line_number":628,"context_line":"                           {\u0027id\u0027: backup_options[\u0027id\u0027],"},{"line_number":629,"context_line":"                            \u0027status\u0027: backup.status})"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_93d6554d","line":626,"updated":"2015-05-22 13:54:11.000000000","message":"exist \u003d\u003e exists","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b696956a00680db146463d5f180e837e2b84db79","unresolved":false,"context_lines":[{"line_number":623,"context_line":""},{"line_number":624,"context_line":"            else:"},{"line_number":625,"context_line":"                if backup.status not in (\u0027available\u0027, \u0027error\u0027, \u0027deleted\u0027):"},{"line_number":626,"context_line":"                    msg \u003d (_(\"Backup %(id)s exist in DB with status %(status)s\""},{"line_number":627,"context_line":"                             \" so we can\u0027t update it\") %"},{"line_number":628,"context_line":"                           {\u0027id\u0027: backup_options[\u0027id\u0027],"},{"line_number":629,"context_line":"                            \u0027status\u0027: backup.status})"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_86282d9b","line":626,"in_reply_to":"7a016987_93d6554d","updated":"2015-05-26 11:40:27.000000000","message":"Done","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e3eba4fbf427fc0da215abfa30a531fef9b46a24","unresolved":false,"context_lines":[{"line_number":624,"context_line":"            else:"},{"line_number":625,"context_line":"                if backup.status not in (\u0027available\u0027, \u0027error\u0027, \u0027deleted\u0027):"},{"line_number":626,"context_line":"                    msg \u003d (_(\"Backup %(id)s exist in DB with status %(status)s\""},{"line_number":627,"context_line":"                             \" so we can\u0027t update it\") %"},{"line_number":628,"context_line":"                           {\u0027id\u0027: backup_options[\u0027id\u0027],"},{"line_number":629,"context_line":"                            \u0027status\u0027: backup.status})"},{"line_number":630,"context_line":"                    raise exception.InvalidBackup(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_35083791","line":627,"updated":"2015-05-15 15:07:34.000000000","message":"What does this indicate if the backup is in a different status?","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aa117b8d67e9ebbc6343945c4aa1f59aae706da8","unresolved":false,"context_lines":[{"line_number":624,"context_line":"            else:"},{"line_number":625,"context_line":"                if backup.status not in (\u0027available\u0027, \u0027error\u0027, \u0027deleted\u0027):"},{"line_number":626,"context_line":"                    msg \u003d (_(\"Backup %(id)s exist in DB with status %(status)s\""},{"line_number":627,"context_line":"                             \" so we can\u0027t update it\") %"},{"line_number":628,"context_line":"                           {\u0027id\u0027: backup_options[\u0027id\u0027],"},{"line_number":629,"context_line":"                            \u0027status\u0027: backup.status})"},{"line_number":630,"context_line":"                    raise exception.InvalidBackup(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_7f2685cf","line":627,"in_reply_to":"7a016987_35083791","updated":"2015-05-15 19:12:41.000000000","message":"Ok, now that you say it like that I realize that the message is not clear. XD\n\nWhat about: \"Backup with id %(id) already in DB, but with status %(status)s, so we can\u0027t overwrite it\"?","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"6899787b6805bc19311af9843601352052ccc89f","unresolved":false,"context_lines":[{"line_number":634,"context_line":"                self.db.backup_destroy(context, backup_id, erase_from_db\u003dTrue)"},{"line_number":635,"context_line":"                backup_id \u003d backup_options[\u0027id\u0027]"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"            self.db.backup_update(context, backup_id, backup_options)"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"            # Verify backup"},{"line_number":640,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_bfe9b61c","line":637,"updated":"2015-05-15 07:02:36.000000000","message":"If we destory the backup at else block, will there raise exception.BackupNotFound when backup update?","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"7b6713ef971b3e1a385c8e5054bde2319ccfc3b6","unresolved":false,"context_lines":[{"line_number":634,"context_line":"                self.db.backup_destroy(context, backup_id, erase_from_db\u003dTrue)"},{"line_number":635,"context_line":"                backup_id \u003d backup_options[\u0027id\u0027]"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"            self.db.backup_update(context, backup_id, backup_options)"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"            # Verify backup"},{"line_number":640,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_1524290b","line":637,"in_reply_to":"7a016987_62ede18a","updated":"2015-05-15 08:57:07.000000000","message":"Emm, this routine you said is right. Because this old backup id is not same with imported backup id. So the destory with erase_from_db\u003dTrue is safety and will not raise exception in db update process. Thanks for the reply.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"26d952a48df984652a36b6dbcfb3ceb448ae2fe2","unresolved":false,"context_lines":[{"line_number":634,"context_line":"                self.db.backup_destroy(context, backup_id, erase_from_db\u003dTrue)"},{"line_number":635,"context_line":"                backup_id \u003d backup_options[\u0027id\u0027]"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"            self.db.backup_update(context, backup_id, backup_options)"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"            # Verify backup"},{"line_number":640,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_62ede18a","line":637,"in_reply_to":"7a016987_bfe9b61c","updated":"2015-05-15 08:15:05.000000000","message":"If backup was previously deleted from the DB (status \u003d deleted) backup_update will not raise BackupNotFound because it also updates deleted backups (that\u0027s why we need to check on line 617 for deleted backups, if we don\u0027t check for deleted we could get an Integrity exception because the backup ID already exists in the DB).\nHere\u0027s where backup_update query which includes deleted backups: https://github.com/openstack/cinder/blob/master/cinder/db/sqlalchemy/api.py#L3268","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"34479edf4e7723197afe2ba400e851437843ee2a","unresolved":false,"context_lines":[{"line_number":625,"context_line":"            else:"},{"line_number":626,"context_line":"                # We\u0027ll only allow to import already deleted backups"},{"line_number":627,"context_line":"                if backup.status !\u003d \u0027deleted\u0027:"},{"line_number":628,"context_line":"                    msg \u003d (_(\"Backup %(id)s already exist in DB with status \""},{"line_number":629,"context_line":"                             \"%(status)s\") % {\u0027id\u0027: backup_options[\u0027id\u0027],"},{"line_number":630,"context_line":"                                              \u0027status\u0027: backup.status})"},{"line_number":631,"context_line":"                    self.db.backup_update(context, backup_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_554d9d3c","line":628,"updated":"2015-05-29 09:26:03.000000000","message":"s/exist/exists/  (as Accela Zhao indicated in changeset 1).","commit_id":"29ab16591cd785326009cf2e06f0a7f9166dd34f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7d339b87dcc2ff572333e2e7146066c2752f5fa2","unresolved":false,"context_lines":[{"line_number":625,"context_line":"            else:"},{"line_number":626,"context_line":"                # We\u0027ll only allow to import already deleted backups"},{"line_number":627,"context_line":"                if backup.status !\u003d \u0027deleted\u0027:"},{"line_number":628,"context_line":"                    msg \u003d (_(\"Backup %(id)s already exist in DB with status \""},{"line_number":629,"context_line":"                             \"%(status)s\") % {\u0027id\u0027: backup_options[\u0027id\u0027],"},{"line_number":630,"context_line":"                                              \u0027status\u0027: backup.status})"},{"line_number":631,"context_line":"                    self.db.backup_update(context, backup_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_1d386b9f","line":628,"in_reply_to":"7a016987_554d9d3c","updated":"2015-05-29 10:55:04.000000000","message":"I was sure I\u0027d done this already when I replied done to Accela\u0027s comment.\n\nThanks for catching this! It\u0027s rude to say \"Done\" and not actually do it.  XD","commit_id":"29ab16591cd785326009cf2e06f0a7f9166dd34f"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5dec801ea5c23f39d20758bf9fd50262e1a1515","unresolved":false,"context_lines":[{"line_number":626,"context_line":"                # created by the API and change its ID to the one being"},{"line_number":627,"context_line":"                # imported."},{"line_number":628,"context_line":"                dst_backup \u003d backup"},{"line_number":629,"context_line":"                self.db.backup_update(context, dst_backup.id,"},{"line_number":630,"context_line":"                                      {\u0027id\u0027: backup_options[\u0027id\u0027]})"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_bf55a950","line":629,"updated":"2015-06-24 10:55:27.000000000","message":"Maybe this needs a new static method in object? I\u0027m not in favor of the idea of changing ID. What if another backup operation is in progress while we change it?","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d384feb5018cc5c21984ddb51b13b60c9b6109b3","unresolved":false,"context_lines":[{"line_number":626,"context_line":"                # created by the API and change its ID to the one being"},{"line_number":627,"context_line":"                # imported."},{"line_number":628,"context_line":"                dst_backup \u003d backup"},{"line_number":629,"context_line":"                self.db.backup_update(context, dst_backup.id,"},{"line_number":630,"context_line":"                                      {\u0027id\u0027: backup_options[\u0027id\u0027]})"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_e6e3a1d6","line":629,"in_reply_to":"fa32b979_bf55a950","updated":"2015-06-24 14:15:46.000000000","message":"Agree.  Maybe a new method like update_id(), but I believe you could try:\n\nbackup \u003d objects.Backup.get_by_id(context, dst_backup.id)\nbackup.id \u003d backup_options[\u0027id\u0027]\nbackup.save()","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5e0767d742ff8e31b9128ef6ecc9378549e2935","unresolved":false,"context_lines":[{"line_number":626,"context_line":"                # created by the API and change its ID to the one being"},{"line_number":627,"context_line":"                # imported."},{"line_number":628,"context_line":"                dst_backup \u003d backup"},{"line_number":629,"context_line":"                self.db.backup_update(context, dst_backup.id,"},{"line_number":630,"context_line":"                                      {\u0027id\u0027: backup_options[\u0027id\u0027]})"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_a3782bc5","line":629,"in_reply_to":"fa32b979_bf55a950","updated":"2015-06-24 13:35:58.000000000","message":"The backup we are changing is the one that was just created by the API for the import, so it\u0027s not yet a backup with data or anything, just a record in the DB. So   I don\u0027t see who would be changing that object.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"da420b6e9fb69d594f78caa1987331be0ced146e","unresolved":false,"context_lines":[{"line_number":626,"context_line":"                # created by the API and change its ID to the one being"},{"line_number":627,"context_line":"                # imported."},{"line_number":628,"context_line":"                dst_backup \u003d backup"},{"line_number":629,"context_line":"                self.db.backup_update(context, dst_backup.id,"},{"line_number":630,"context_line":"                                      {\u0027id\u0027: backup_options[\u0027id\u0027]})"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_feffa1ff","line":629,"in_reply_to":"fa32b979_d353b6e0","updated":"2015-06-24 16:29:48.000000000","message":"Ok.  We are trying to move away from doing direct db calls by using oslo_versionedobjects.  Leaving it specifically for this case goes against what we are trying to do.  We should create some static method in the backup object (maybe you can put the entire try/except logic in the method) and just watch and gate patches that come in that use this method in the wrong way.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"db9022fa2b1f5981d797da6f5b17e3a3df6aa0bb","unresolved":false,"context_lines":[{"line_number":626,"context_line":"                # created by the API and change its ID to the one being"},{"line_number":627,"context_line":"                # imported."},{"line_number":628,"context_line":"                dst_backup \u003d backup"},{"line_number":629,"context_line":"                self.db.backup_update(context, dst_backup.id,"},{"line_number":630,"context_line":"                                      {\u0027id\u0027: backup_options[\u0027id\u0027]})"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_d353b6e0","line":629,"in_reply_to":"fa32b979_e6e3a1d6","updated":"2015-06-24 16:05:54.000000000","message":"@Thang: That won\u0027t work, as explained in the comment at line 620.\n\nDoing backup.id \u003d something will dirty the id the same as doing backup.update, and then when you go to save it will search by that ID in the DB for the update, but no record with that new ID exists, so it will fail.\n\nI don\u0027t mind creating an object method like the one you guys mention, but I think that will open the door to people trying to change the ID to resources that have actual data in other patches and that would could create havoc.\n\nThis way the ID modification is contained to 1 case where we know for a fact that the resource/record has no actual data and no history or relations with other tables.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ba3c6bf9173dc31b83219dbb9af6b263d0aa4a0b","unresolved":false,"context_lines":[{"line_number":626,"context_line":"                # created by the API and change its ID to the one being"},{"line_number":627,"context_line":"                # imported."},{"line_number":628,"context_line":"                dst_backup \u003d backup"},{"line_number":629,"context_line":"                self.db.backup_update(context, dst_backup.id,"},{"line_number":630,"context_line":"                                      {\u0027id\u0027: backup_options[\u0027id\u0027]})"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_3fcf4357","line":629,"in_reply_to":"fa32b979_feffa1ff","updated":"2015-06-24 17:22:27.000000000","message":"I\u0027ll look for a oslo_versionedobjects solution.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5dec801ea5c23f39d20758bf9fd50262e1a1515","unresolved":false,"context_lines":[{"line_number":641,"context_line":""},{"line_number":642,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":643,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":644,"context_line":"                self.db.backup_destroy(context, backup.id, erase_from_db\u003dTrue)"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"            # Update the database with imported data"},{"line_number":647,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_3f13d9b9","line":644,"updated":"2015-06-24 10:55:27.000000000","message":"ditto","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d384feb5018cc5c21984ddb51b13b60c9b6109b3","unresolved":false,"context_lines":[{"line_number":641,"context_line":""},{"line_number":642,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":643,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":644,"context_line":"                self.db.backup_destroy(context, backup.id, erase_from_db\u003dTrue)"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"            # Update the database with imported data"},{"line_number":647,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_46184dd3","line":644,"in_reply_to":"fa32b979_3f13d9b9","updated":"2015-06-24 14:15:46.000000000","message":"+1","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5e0767d742ff8e31b9128ef6ecc9378549e2935","unresolved":false,"context_lines":[{"line_number":641,"context_line":""},{"line_number":642,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":643,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":644,"context_line":"                self.db.backup_destroy(context, backup.id, erase_from_db\u003dTrue)"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"            # Update the database with imported data"},{"line_number":647,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_6355431c","line":644,"in_reply_to":"fa32b979_3f13d9b9","updated":"2015-06-24 13:35:58.000000000","message":"Same here, the backup we are deleting has no real data in it, it was just created by the API when import was called.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"db9022fa2b1f5981d797da6f5b17e3a3df6aa0bb","unresolved":false,"context_lines":[{"line_number":641,"context_line":""},{"line_number":642,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":643,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":644,"context_line":"                self.db.backup_destroy(context, backup.id, erase_from_db\u003dTrue)"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"            # Update the database with imported data"},{"line_number":647,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_b33152a4","line":644,"in_reply_to":"fa32b979_46184dd3","updated":"2015-06-24 16:05:54.000000000","message":"If you want I can add an optional argument to the delete method in Backup object that allows deleting it from the DB.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"da420b6e9fb69d594f78caa1987331be0ced146e","unresolved":false,"context_lines":[{"line_number":641,"context_line":""},{"line_number":642,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":643,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":644,"context_line":"                self.db.backup_destroy(context, backup.id, erase_from_db\u003dTrue)"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"            # Update the database with imported data"},{"line_number":647,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_3911a352","line":644,"in_reply_to":"fa32b979_b33152a4","updated":"2015-06-24 16:29:48.000000000","message":"Sounds fine.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"82ec663ae754fd9d9761df3c7ff459fa2452b83c","unresolved":false,"context_lines":[{"line_number":641,"context_line":""},{"line_number":642,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":643,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":644,"context_line":"                self.db.backup_destroy(context, backup.id, erase_from_db\u003dTrue)"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"            # Update the database with imported data"},{"line_number":647,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_748b86c3","line":644,"in_reply_to":"fa32b979_b33152a4","updated":"2015-06-25 13:20:11.000000000","message":"That\u0027s the right approach.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"6488f401269b34bbe8030c40445dc71e88e64a71","unresolved":false,"context_lines":[{"line_number":640,"context_line":""},{"line_number":641,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":642,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":643,"context_line":"                backup.destroy()"},{"line_number":644,"context_line":""},{"line_number":645,"context_line":"            # Update the database with imported data"},{"line_number":646,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba3cc151_9567a024","line":643,"updated":"2015-07-01 06:54:27.000000000","message":"Go to backup/api.py, there are following codes in import_record():\n        self.backup_rpcapi.import_record(context,\n                                         first_host,\n                                         backup,\n                                         backup_service,\n                                         backup_url,\n                                         hosts)\n\n        return backup\n\nIs it ok to destroy backup here?","commit_id":"49733fe34d2302d12a94aa149c2c0e1bd098f3d0"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"0a196f40348844eb9cbe7ea4d37ed3bd7f3db977","unresolved":false,"context_lines":[{"line_number":640,"context_line":""},{"line_number":641,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":642,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":643,"context_line":"                backup.destroy()"},{"line_number":644,"context_line":""},{"line_number":645,"context_line":"            # Update the database with imported data"},{"line_number":646,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba3cc151_06a22199","line":643,"in_reply_to":"ba3cc151_9391eef3","updated":"2015-07-02 23:55:42.000000000","message":"Gorka, \nWhat I am concerned here is that the following part calls manager.import_record.\n        self.backup_rpcapi.import_record(context,\n                                         first_host,\n                                         backup,\n                                         backup_service,\n                                         backup_url,\n                                         hosts)\n\n        return backup\n\nShould it be changed to \n        backup \u003d self.backup_rpcapi.import_record(context,\n                                         first_host,\n                                         backup,\n                                         backup_service,\n                                         backup_url,\n                                         hosts)\n\n        return backup\n\nSo that after cmd runs, it will not return a backup with wrong id.","commit_id":"49733fe34d2302d12a94aa149c2c0e1bd098f3d0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"355de7fb963582c77f8742381396efd52d474d4f","unresolved":false,"context_lines":[{"line_number":640,"context_line":""},{"line_number":641,"context_line":"                # If we are updating existing backup record, we have to"},{"line_number":642,"context_line":"                # delete from the DB the backup record created by the API"},{"line_number":643,"context_line":"                backup.destroy()"},{"line_number":644,"context_line":""},{"line_number":645,"context_line":"            # Update the database with imported data"},{"line_number":646,"context_line":"            dst_backup.update(backup_options)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba3cc151_9391eef3","line":643,"in_reply_to":"ba3cc151_9567a024","updated":"2015-07-02 16:27:02.000000000","message":"My first instinct was to tell you that this is necessary, even if it is not ideal, because the alternatives are leaving it in the DB with one of these statuses:\n\n- deleted: The user won\u0027t be able to tell the difference from real delete\n- creating: It would look like we are still working on the import\n- error: Would be inaccurate since we were able to import the record, even if it\u0027s on another ID.\n- available: Also inaccurate since this backup entry has no backup data.\n\nAnd that I thought any of those might be even worse.\n\nBut then I thought that there should be a better way to do it, and after giving it some thought I have seen that there is, but I\u0027m not sure it\u0027s going to be accepted by the community. And it\u0027s quite the rework, so I would like to hear opinions before doing the work, so I should create specs for it.\n\nThe highlight is that right now the decoding of the metadata is done in the drivers, so it\u0027s not accessible from the API node were the temporary backup record is created.  To fix this temporary record thing we would need to change this so that we can decode the metadata from the API and then check the ID to decide if we return an error, create a new Backup entry with specified ID, or update entry on the DB.\n\nOn the Backup manager the import/export metadata should get additional information from the drivers and use it together with basic data from Backup methods to perform the operation.","commit_id":"49733fe34d2302d12a94aa149c2c0e1bd098f3d0"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"6488f401269b34bbe8030c40445dc71e88e64a71","unresolved":false,"context_lines":[{"line_number":662,"context_line":"                                              six.text_type(err))"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"            LOG.info(_LI(\u0027Import record id %s metadata from driver \u0027"},{"line_number":665,"context_line":"                         \u0027finished.\u0027), backup.id)"},{"line_number":666,"context_line":""},{"line_number":667,"context_line":"    def reset_status(self, context, backup, status):"},{"line_number":668,"context_line":"        \"\"\"Reset volume backup status."}],"source_content_type":"text/x-python","patch_set":11,"id":"ba3cc151_d5e1a89b","line":665,"updated":"2015-07-01 06:54:27.000000000","message":"Here use backup.id. I think it is not correct, it should be dst_backup.id instead.","commit_id":"49733fe34d2302d12a94aa149c2c0e1bd098f3d0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"355de7fb963582c77f8742381396efd52d474d4f","unresolved":false,"context_lines":[{"line_number":662,"context_line":"                                              six.text_type(err))"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"            LOG.info(_LI(\u0027Import record id %s metadata from driver \u0027"},{"line_number":665,"context_line":"                         \u0027finished.\u0027), backup.id)"},{"line_number":666,"context_line":""},{"line_number":667,"context_line":"    def reset_status(self, context, backup, status):"},{"line_number":668,"context_line":"        \"\"\"Reset volume backup status."}],"source_content_type":"text/x-python","patch_set":11,"id":"ba3cc151_6d9c94c0","line":665,"in_reply_to":"ba3cc151_d5e1a89b","updated":"2015-07-02 16:27:02.000000000","message":"Thanks","commit_id":"49733fe34d2302d12a94aa149c2c0e1bd098f3d0"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"5ae342d655483547dfeb0f7d2035b464fa32eccc","unresolved":false,"context_lines":[{"line_number":605,"context_line":"            if backup_id !\u003d backup.id:"},{"line_number":606,"context_line":"                msg \u003d (_(\u0027Trying to import backup metadata from id %(meta_id)s\u0027"},{"line_number":607,"context_line":"                         \u0027 into backup %(id)s.\u0027) %"},{"line_number":608,"context_line":"                       {\u0027meta_id\u0027: backup_id, \u0027id\u0027: backup.id})"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"            # Overwrite some fields"},{"line_number":611,"context_line":"            backup_options[\u0027status\u0027] \u003d \u0027available\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a41bdd9_8d832904","line":608,"updated":"2015-07-14 02:22:57.000000000","message":"I feel this block code is unnecessary since you have made the backup id consistent in API. Is there other case that can make the backup id inconsistent?","commit_id":"b39cb00c00851a7894c6e3c520ba9ff91cce3b9e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"198898cfecaaeb3065a0813f12dff313d0c00d38","unresolved":false,"context_lines":[{"line_number":605,"context_line":"            if backup_id !\u003d backup.id:"},{"line_number":606,"context_line":"                msg \u003d (_(\u0027Trying to import backup metadata from id %(meta_id)s\u0027"},{"line_number":607,"context_line":"                         \u0027 into backup %(id)s.\u0027) %"},{"line_number":608,"context_line":"                       {\u0027meta_id\u0027: backup_id, \u0027id\u0027: backup.id})"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"            # Overwrite some fields"},{"line_number":611,"context_line":"            backup_options[\u0027status\u0027] \u003d \u0027available\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a41bdd9_dd71b645","line":608,"in_reply_to":"9a41bdd9_4f9dfdba","updated":"2015-07-14 18:10:31.000000000","message":"Oh boy!  That was supposed to update the DB entry and raise an exception, but apparently I deleted too much in the refactoring.  Good catch!!  Thanks!!","commit_id":"b39cb00c00851a7894c6e3c520ba9ff91cce3b9e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3484cb63c546323f2bccd096609d98b17338b038","unresolved":false,"context_lines":[{"line_number":605,"context_line":"            if backup_id !\u003d backup.id:"},{"line_number":606,"context_line":"                msg \u003d (_(\u0027Trying to import backup metadata from id %(meta_id)s\u0027"},{"line_number":607,"context_line":"                         \u0027 into backup %(id)s.\u0027) %"},{"line_number":608,"context_line":"                       {\u0027meta_id\u0027: backup_id, \u0027id\u0027: backup.id})"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"            # Overwrite some fields"},{"line_number":611,"context_line":"            backup_options[\u0027status\u0027] \u003d \u0027available\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a41bdd9_e519a47a","line":608,"in_reply_to":"9a41bdd9_8d832904","updated":"2015-07-14 08:21:54.000000000","message":"If you receive the request from an API Node with old code you may end up with the wrong backup id.\n\nUsually I wouldn\u0027t bother with this kind of check, but in this case if the ID is not correct the record is useless, because some backup backends won\u0027t be able to access their data and incremental will be broken.\n\nSince it\u0027s imperative to have the right ID, I think it\u0027s a good idea to check it.  Better safe than sorry.","commit_id":"b39cb00c00851a7894c6e3c520ba9ff91cce3b9e"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"33e4973d4667b67a559934382b86802a0c7675a3","unresolved":false,"context_lines":[{"line_number":605,"context_line":"            if backup_id !\u003d backup.id:"},{"line_number":606,"context_line":"                msg \u003d (_(\u0027Trying to import backup metadata from id %(meta_id)s\u0027"},{"line_number":607,"context_line":"                         \u0027 into backup %(id)s.\u0027) %"},{"line_number":608,"context_line":"                       {\u0027meta_id\u0027: backup_id, \u0027id\u0027: backup.id})"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"            # Overwrite some fields"},{"line_number":611,"context_line":"            backup_options[\u0027status\u0027] \u003d \u0027available\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a41bdd9_0a47babc","line":608,"in_reply_to":"9a41bdd9_e519a47a","updated":"2015-07-15 06:52:44.000000000","message":"Got it.","commit_id":"b39cb00c00851a7894c6e3c520ba9ff91cce3b9e"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"4125b55296693615ea3efe98f6a13a1df5f212c6","unresolved":false,"context_lines":[{"line_number":605,"context_line":"            if backup_id !\u003d backup.id:"},{"line_number":606,"context_line":"                msg \u003d (_(\u0027Trying to import backup metadata from id %(meta_id)s\u0027"},{"line_number":607,"context_line":"                         \u0027 into backup %(id)s.\u0027) %"},{"line_number":608,"context_line":"                       {\u0027meta_id\u0027: backup_id, \u0027id\u0027: backup.id})"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"            # Overwrite some fields"},{"line_number":611,"context_line":"            backup_options[\u0027status\u0027] \u003d \u0027available\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a41bdd9_4f9dfdba","line":608,"in_reply_to":"9a41bdd9_e519a47a","updated":"2015-07-14 13:47:48.000000000","message":"It looks like you\u0027ve missed the logging message (LOG.info(...)) otherwise what sense in that msg.","commit_id":"b39cb00c00851a7894c6e3c520ba9ff91cce3b9e"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7c47f120189372cf967a1513bb6df03a8656d0ac","unresolved":false,"context_lines":[{"line_number":3187,"context_line":""},{"line_number":3188,"context_line":""},{"line_number":3189,"context_line":"@require_context"},{"line_number":3190,"context_line":"def backup_get(context, backup_id, read_deleted\u003dNone, project_only\u003dTrue):"},{"line_number":3191,"context_line":"    result \u003d model_query(context, models.Backup, read_deleted\u003dread_deleted,"},{"line_number":3192,"context_line":"                         project_only\u003dproject_only).\\"},{"line_number":3193,"context_line":"        filter_by(id\u003dbackup_id).\\"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_5370dda3","line":3190,"updated":"2015-05-22 13:54:11.000000000","message":"I think this creates certain inconsistency since other xxx_get APIs doesn\u0027t have the newly added argument options.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b696956a00680db146463d5f180e837e2b84db79","unresolved":false,"context_lines":[{"line_number":3187,"context_line":""},{"line_number":3188,"context_line":""},{"line_number":3189,"context_line":"@require_context"},{"line_number":3190,"context_line":"def backup_get(context, backup_id, read_deleted\u003dNone, project_only\u003dTrue):"},{"line_number":3191,"context_line":"    result \u003d model_query(context, models.Backup, read_deleted\u003dread_deleted,"},{"line_number":3192,"context_line":"                         project_only\u003dproject_only).\\"},{"line_number":3193,"context_line":"        filter_by(id\u003dbackup_id).\\"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_a10af31d","line":3190,"in_reply_to":"7a016987_5370dda3","updated":"2015-05-26 11:40:27.000000000","message":"The alternative is to create a new method that allows reading deleted and non-project records, which I think it\u0027s worse.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"75dbb67e29ebd4f5dc5b8f8c8b207d6b948ada9f","unresolved":false,"context_lines":[{"line_number":3187,"context_line":""},{"line_number":3188,"context_line":""},{"line_number":3189,"context_line":"@require_context"},{"line_number":3190,"context_line":"def backup_get(context, backup_id, read_deleted\u003dNone, project_only\u003dTrue):"},{"line_number":3191,"context_line":"    result \u003d model_query(context, models.Backup, read_deleted\u003dread_deleted,"},{"line_number":3192,"context_line":"                         project_only\u003dproject_only).\\"},{"line_number":3193,"context_line":"        filter_by(id\u003dbackup_id).\\"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_da9ace13","line":3190,"in_reply_to":"7a016987_a10af31d","updated":"2015-05-29 15:44:52.000000000","message":"Got your point.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7c47f120189372cf967a1513bb6df03a8656d0ac","unresolved":false,"context_lines":[{"line_number":3279,"context_line":""},{"line_number":3280,"context_line":""},{"line_number":3281,"context_line":"@require_admin_context"},{"line_number":3282,"context_line":"def backup_destroy(context, backup_id, erase_from_db\u003dFalse):"},{"line_number":3283,"context_line":"        backup \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":3284,"context_line":"        if erase_from_db:"},{"line_number":3285,"context_line":"            backup.delete()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_336c41cd","line":3282,"updated":"2015-05-22 13:54:11.000000000","message":"Ditto.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b696956a00680db146463d5f180e837e2b84db79","unresolved":false,"context_lines":[{"line_number":3279,"context_line":""},{"line_number":3280,"context_line":""},{"line_number":3281,"context_line":"@require_admin_context"},{"line_number":3282,"context_line":"def backup_destroy(context, backup_id, erase_from_db\u003dFalse):"},{"line_number":3283,"context_line":"        backup \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":3284,"context_line":"        if erase_from_db:"},{"line_number":3285,"context_line":"            backup.delete()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_c11e0f5e","line":3282,"in_reply_to":"7a016987_336c41cd","updated":"2015-05-26 11:40:27.000000000","message":"Here the alternatives are: create a new method or leave the temporary records that were never used in the DB as deleted.\n\nI think both alternatives are less preferable.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"75dbb67e29ebd4f5dc5b8f8c8b207d6b948ada9f","unresolved":false,"context_lines":[{"line_number":3279,"context_line":""},{"line_number":3280,"context_line":""},{"line_number":3281,"context_line":"@require_admin_context"},{"line_number":3282,"context_line":"def backup_destroy(context, backup_id, erase_from_db\u003dFalse):"},{"line_number":3283,"context_line":"        backup \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":3284,"context_line":"        if erase_from_db:"},{"line_number":3285,"context_line":"            backup.delete()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_ba9b9216","line":3282,"in_reply_to":"7a016987_c11e0f5e","updated":"2015-05-29 15:44:52.000000000","message":"Got your point.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"dea9a5a23a433aea87906f0298c1bdf4bd23f303","unresolved":false,"context_lines":[{"line_number":3282,"context_line":"def backup_destroy(context, backup_id, erase_from_db\u003dFalse):"},{"line_number":3283,"context_line":"        backup \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":3284,"context_line":"        if erase_from_db:"},{"line_number":3285,"context_line":"            backup.delete()"},{"line_number":3286,"context_line":"        else:"},{"line_number":3287,"context_line":"            backup.update({\u0027status\u0027: \u0027deleted\u0027,"},{"line_number":3288,"context_line":"                           \u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_f5829251","line":3285,"updated":"2015-06-03 10:42:06.000000000","message":"backup.delete won\u0027t delete data from DB.  It will invoke this https://github.com/openstack/cinder/blob/master/cinder/db/sqlalchemy/models.py#L47 method","commit_id":"65aa6e082520c0b78f33a439298927cb374d03d1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b5b3b8652375a9913751db877db013f942d23356","unresolved":false,"context_lines":[{"line_number":3282,"context_line":"def backup_destroy(context, backup_id, erase_from_db\u003dFalse):"},{"line_number":3283,"context_line":"        backup \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":3284,"context_line":"        if erase_from_db:"},{"line_number":3285,"context_line":"            backup.delete()"},{"line_number":3286,"context_line":"        else:"},{"line_number":3287,"context_line":"            backup.update({\u0027status\u0027: \u0027deleted\u0027,"},{"line_number":3288,"context_line":"                           \u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afb71cf_45d26f5d","line":3285,"in_reply_to":"3afb71cf_c19b0c65","updated":"2015-06-10 07:50:01.000000000","message":"No problem, it was a really good question. :)","commit_id":"65aa6e082520c0b78f33a439298927cb374d03d1"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"7b1054ddd7d9f88e5e46e08414a7e608192805d6","unresolved":false,"context_lines":[{"line_number":3282,"context_line":"def backup_destroy(context, backup_id, erase_from_db\u003dFalse):"},{"line_number":3283,"context_line":"        backup \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":3284,"context_line":"        if erase_from_db:"},{"line_number":3285,"context_line":"            backup.delete()"},{"line_number":3286,"context_line":"        else:"},{"line_number":3287,"context_line":"            backup.update({\u0027status\u0027: \u0027deleted\u0027,"},{"line_number":3288,"context_line":"                           \u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afb71cf_c19b0c65","line":3285,"in_reply_to":"5afe65bd_f0d06055","updated":"2015-06-09 18:51:04.000000000","message":"You\u0027re right. I\u0027m sorry if confused you","commit_id":"65aa6e082520c0b78f33a439298927cb374d03d1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"2ff115b5b30f4ce78d80e1b1f4e7af08f2162085","unresolved":false,"context_lines":[{"line_number":3282,"context_line":"def backup_destroy(context, backup_id, erase_from_db\u003dFalse):"},{"line_number":3283,"context_line":"        backup \u003d model_query(context, models.Backup).filter_by(id\u003dbackup_id)"},{"line_number":3284,"context_line":"        if erase_from_db:"},{"line_number":3285,"context_line":"            backup.delete()"},{"line_number":3286,"context_line":"        else:"},{"line_number":3287,"context_line":"            backup.update({\u0027status\u0027: \u0027deleted\u0027,"},{"line_number":3288,"context_line":"                           \u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_f0d06055","line":3285,"in_reply_to":"5afe65bd_f5829251","updated":"2015-06-03 11:10:41.000000000","message":"We are getting a Query, not a ModelBase object, in backup so when we do a delete we are really calling: https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/orm/query.py#L2736\n\nWhich means we will delete the object. The patch has a test that confirms this, it\u0027s called test_backup_destroy_erase_from_db in https://review.openstack.org/#/c/183222/4/cinder/tests/unit/test_db_api.py","commit_id":"65aa6e082520c0b78f33a439298927cb374d03d1"}],"cinder/objects/backup.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5dec801ea5c23f39d20758bf9fd50262e1a1515","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        return backup"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    @base.remotable_classmethod"},{"line_number":84,"context_line":"    def get_by_id(cls, context, id, *args, **kwargs):"},{"line_number":85,"context_line":"        db_backup \u003d db.backup_get(context, id, *args, **kwargs)"},{"line_number":86,"context_line":"        return cls._from_db_object(context, cls(context), db_backup)"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_5c7a6b89","line":84,"updated":"2015-06-24 10:55:27.000000000","message":"Why *args and **kwargs when we can just pass read_deleted and project_only explicitly? This reduces readability IMO.\n\nOr maybe we need another method like get_by_id_project_only?","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d384feb5018cc5c21984ddb51b13b60c9b6109b3","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        return backup"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    @base.remotable_classmethod"},{"line_number":84,"context_line":"    def get_by_id(cls, context, id, *args, **kwargs):"},{"line_number":85,"context_line":"        db_backup \u003d db.backup_get(context, id, *args, **kwargs)"},{"line_number":86,"context_line":"        return cls._from_db_object(context, cls(context), db_backup)"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_9c78e8c5","line":84,"in_reply_to":"fa32b979_5c7a6b89","updated":"2015-06-24 14:15:46.000000000","message":"+1.  We should explicitly pass in the arguments.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5e0767d742ff8e31b9128ef6ecc9378549e2935","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        return backup"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    @base.remotable_classmethod"},{"line_number":84,"context_line":"    def get_by_id(cls, context, id, *args, **kwargs):"},{"line_number":85,"context_line":"        db_backup \u003d db.backup_get(context, id, *args, **kwargs)"},{"line_number":86,"context_line":"        return cls._from_db_object(context, cls(context), db_backup)"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_866655b9","line":84,"in_reply_to":"fa32b979_5c7a6b89","updated":"2015-06-24 13:35:58.000000000","message":"I have no problem adding that, but I thought this would avoid changing it in the future if more arguments are added to backup_get.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"82ec663ae754fd9d9761df3c7ff459fa2452b83c","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        return backup"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    @base.remotable_classmethod"},{"line_number":84,"context_line":"    def get_by_id(cls, context, id, *args, **kwargs):"},{"line_number":85,"context_line":"        db_backup \u003d db.backup_get(context, id, *args, **kwargs)"},{"line_number":86,"context_line":"        return cls._from_db_object(context, cls(context), db_backup)"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_54a3e242","line":84,"in_reply_to":"fa32b979_866655b9","updated":"2015-06-25 13:20:11.000000000","message":"The idea of VersionedObjects is also to decouple resources representation from DB representation. You should use meaningful parameters here.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"218e7c1cdd7bc4616cfd4b8694ade929f48305db","unresolved":false,"context_lines":[{"line_number":174,"context_line":"    @base.remotable"},{"line_number":175,"context_line":"    def set_new_id(self, new_id):"},{"line_number":176,"context_line":"        \"\"\"Change the ID of the volume.\"\"\""},{"line_number":177,"context_line":"        with self.obj_as_admin():"},{"line_number":178,"context_line":"            # Update DB"},{"line_number":179,"context_line":"            db.backup_update(self._context, self.id, {\u0027id\u0027: new_id})"},{"line_number":180,"context_line":"            # Update the object ID"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_1688e121","line":177,"updated":"2015-06-29 09:17:25.000000000","message":"As db.sqlalchemy.api.backup_update doesn\u0027t have require_admin_context - I don\u0027t think this line is needed.","commit_id":"68eac247618c8e03e63015199f784508ddef2647"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fb2d6dfa170677440cb1b4fecc1d4c8692700566","unresolved":false,"context_lines":[{"line_number":174,"context_line":"    @base.remotable"},{"line_number":175,"context_line":"    def set_new_id(self, new_id):"},{"line_number":176,"context_line":"        \"\"\"Change the ID of the volume.\"\"\""},{"line_number":177,"context_line":"        with self.obj_as_admin():"},{"line_number":178,"context_line":"            # Update DB"},{"line_number":179,"context_line":"            db.backup_update(self._context, self.id, {\u0027id\u0027: new_id})"},{"line_number":180,"context_line":"            # Update the object ID"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_2c74b19a","line":177,"in_reply_to":"ba3cc151_1688e121","updated":"2015-06-29 13:17:39.000000000","message":"Good catch!  Yank-Paste error  ;-)","commit_id":"68eac247618c8e03e63015199f784508ddef2647"}],"cinder/test.py":[{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d384feb5018cc5c21984ddb51b13b60c9b6109b3","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # datetimes with timezones, so we need our objects to NOT be timezone"},{"line_number":142,"context_line":"        # aware for the tests; otherwise we\u0027ll end up with errors when updating"},{"line_number":143,"context_line":"        # and saving datetime fields: \"TypeError: can\u0027t compare offset-naive"},{"line_number":144,"context_line":"        # and offset-aware datetimes\"."},{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_37053510","line":144,"updated":"2015-06-24 14:15:46.000000000","message":"Instead, you should use - tzinfo\u003diso8601.iso8601.Utc()","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"db9022fa2b1f5981d797da6f5b17e3a3df6aa0bb","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # datetimes with timezones, so we need our objects to NOT be timezone"},{"line_number":142,"context_line":"        # aware for the tests; otherwise we\u0027ll end up with errors when updating"},{"line_number":143,"context_line":"        # and saving datetime fields: \"TypeError: can\u0027t compare offset-naive"},{"line_number":144,"context_line":"        # and offset-aware datetimes\"."},{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_d3f09693","line":144,"in_reply_to":"fa32b979_37053510","updated":"2015-06-24 16:05:54.000000000","message":"Where do you suggest I use that?\n\nBecause the code for patching mysqlalchemy to alter all records read from the DB that are datetime fields would be a lot more complex and difficult to read than this.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"da420b6e9fb69d594f78caa1987331be0ced146e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # datetimes with timezones, so we need our objects to NOT be timezone"},{"line_number":142,"context_line":"        # aware for the tests; otherwise we\u0027ll end up with errors when updating"},{"line_number":143,"context_line":"        # and saving datetime fields: \"TypeError: can\u0027t compare offset-naive"},{"line_number":144,"context_line":"        # and offset-aware datetimes\"."},{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_d9bc3708","line":144,"in_reply_to":"fa32b979_d3f09693","updated":"2015-06-24 16:29:48.000000000","message":"See https://review.openstack.org/#/c/177054/34/cinder/tests/unit/api/contrib/test_volume_actions.py#L778 as an example.  I ran into the same problem updating the cinder volume unit tests.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ba3c6bf9173dc31b83219dbb9af6b263d0aa4a0b","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # datetimes with timezones, so we need our objects to NOT be timezone"},{"line_number":142,"context_line":"        # aware for the tests; otherwise we\u0027ll end up with errors when updating"},{"line_number":143,"context_line":"        # and saving datetime fields: \"TypeError: can\u0027t compare offset-naive"},{"line_number":144,"context_line":"        # and offset-aware datetimes\"."},{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_04df92d6","line":144,"in_reply_to":"fa32b979_d9bc3708","updated":"2015-06-24 17:22:27.000000000","message":"If only it was that easy! The problem is a lot more complicated in these tests.\n\nIn backup import method you decode backup information into a dictionary that then you use to update a Backup object. Till here everything works just fine.\n\nSince a datetime field has been updated in the Backup object when the save method is called you get an exception in oslo versioned object code because before updating it compares values with DB. Which results in comparing tz unaware field (sqlite) with tz aware field (field from Backup object).","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5dec801ea5c23f39d20758bf9fd50262e1a1515","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"},{"line_number":148,"context_line":"        object_fields \u003d objects.base.CinderPersistentObject.fields"},{"line_number":149,"context_line":"        for k, v in object_fields.items():"},{"line_number":150,"context_line":"            if isinstance(v, fields.DateTimeField):"},{"line_number":151,"context_line":"                object_fields[k] \u003d fields.DateTimeField(tzinfo_aware\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_bf5fe93f","line":148,"updated":"2015-06-24 10:55:27.000000000","message":"Okay, this is interesting... We forced to use non-timezone-aware objects in other patches *directly* in object definition. Maybe this is a better solution? Let\u0027s discuss it on today\u0027s meeting.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"82ec663ae754fd9d9761df3c7ff459fa2452b83c","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"},{"line_number":148,"context_line":"        object_fields \u003d objects.base.CinderPersistentObject.fields"},{"line_number":149,"context_line":"        for k, v in object_fields.items():"},{"line_number":150,"context_line":"            if isinstance(v, fields.DateTimeField):"},{"line_number":151,"context_line":"                object_fields[k] \u003d fields.DateTimeField(tzinfo_aware\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_f419d688","line":148,"in_reply_to":"fa32b979_662c71ac","updated":"2015-06-25 13:20:11.000000000","message":"Okay, I think that we got this straighten up at the meeting - approach in this patch is an accepted one.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5e0767d742ff8e31b9128ef6ecc9378549e2935","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"},{"line_number":148,"context_line":"        object_fields \u003d objects.base.CinderPersistentObject.fields"},{"line_number":149,"context_line":"        for k, v in object_fields.items():"},{"line_number":150,"context_line":"            if isinstance(v, fields.DateTimeField):"},{"line_number":151,"context_line":"                object_fields[k] \u003d fields.DateTimeField(tzinfo_aware\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_662c71ac","line":148,"in_reply_to":"fa32b979_bf5fe93f","updated":"2015-06-24 13:35:58.000000000","message":"I\u0027d like to see how it was done there, since most of the other ways I tried wouldn\u0027t work when the backup import method tried to update \u0026 save the backup using the dictionary returned from the decoding of backup_url .","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cea8c0280ffe1ebec9b298a6cdb80310857eb451","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        #"},{"line_number":146,"context_line":"        # Change datetime fields in CinderPersistentObject to be timezone"},{"line_number":147,"context_line":"        # unaware"},{"line_number":148,"context_line":"        object_fields \u003d objects.base.CinderPersistentObject.fields"},{"line_number":149,"context_line":"        for k, v in object_fields.items():"},{"line_number":150,"context_line":"            if isinstance(v, fields.DateTimeField):"},{"line_number":151,"context_line":"                object_fields[k] \u003d fields.DateTimeField(tzinfo_aware\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_88f37b9f","line":148,"in_reply_to":"fa32b979_f419d688","updated":"2015-06-25 19:24:38.000000000","message":"Well, I\u0027ve been working on this and it\u0027s more complicated.  I modified this so that all datetime fields read from sqlite would get the UTC tz added if they didn\u0027t have it, but then when I ran all tests I got lots of errors.\n\nAs it turns out, our DB in general doesn\u0027t use timezones, so we have to fix this on the object side.  I have a patch ready to fix that, independently of this one, and it will include a better explanation of the problem.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"}],"cinder/tests/unit/objects/base.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1a0f5a6417b00c16665783b7a3e002040d5a096b","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    }"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"class TestCinderObject(test_objects.BaseObjectsTestCase):"},{"line_number":34,"context_line":"    \"\"\"Tests obj_get_changes from CinderObject.\"\"\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba3cc151_b4f523cc","line":33,"updated":"2015-06-30 17:22:05.000000000","message":"Aren\u0027t these tests added in \tcinder/tests/unit/objects/test_base.py in base patch?","commit_id":"a2d404c179c436cb44aca85c83e060fd8f67b12d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9c79120b9c876be65bc444a9b5118b1d8e60d74e","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    }"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"class TestCinderObject(test_objects.BaseObjectsTestCase):"},{"line_number":34,"context_line":"    \"\"\"Tests obj_get_changes from CinderObject.\"\"\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba3cc151_d40c6796","line":33,"in_reply_to":"ba3cc151_b4f523cc","updated":"2015-06-30 17:29:07.000000000","message":"Great catch!!\n\nYou are right, this is a leftover from one of the rebases...","commit_id":"a2d404c179c436cb44aca85c83e060fd8f67b12d"}],"cinder/tests/unit/test_backup.py":[{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7c47f120189372cf967a1513bb6df03a8656d0ac","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if k.startswith(\u0027_\u0027) or k in (\u0027updated_at\u0027, \u0027id\u0027):"},{"line_number":146,"context_line":"                del reference[k]"},{"line_number":147,"context_line":"                del result[k]"},{"line_number":148,"context_line":"        self.assertEqual(reference, result)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"class BackupTestCase(BaseBackupTest):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_13a26599","line":148,"updated":"2015-05-22 13:54:11.000000000","message":"Since this is a tool method, it should return true or false rather than end with an assert. If you end it with \"assertEqual\", a better method name should be \"assert_backups_equal\".","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b696956a00680db146463d5f180e837e2b84db79","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if k.startswith(\u0027_\u0027) or k in (\u0027updated_at\u0027, \u0027id\u0027):"},{"line_number":146,"context_line":"                del reference[k]"},{"line_number":147,"context_line":"                del result[k]"},{"line_number":148,"context_line":"        self.assertEqual(reference, result)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"class BackupTestCase(BaseBackupTest):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_01ec2728","line":148,"in_reply_to":"7a016987_13a26599","updated":"2015-05-26 11:40:27.000000000","message":"Done","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5dec801ea5c23f39d20758bf9fd50262e1a1515","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        backup.create()"},{"line_number":139,"context_line":"        return backup"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def _assert_backups_equal(self, reference, result):"},{"line_number":142,"context_line":"        reference \u003d vars(reference)"},{"line_number":143,"context_line":"        result \u003d vars(result)"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_5c7fcbdf","line":141,"updated":"2015-06-24 10:55:27.000000000","message":"Maybe you can try this method and it\u0027s DONTCARE? https://github.com/openstack/cinder/blob/master/cinder/test.py#L319-L374  Backup object has CinderObjectDictCompat, so it should be fine.","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5e0767d742ff8e31b9128ef6ecc9378549e2935","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        backup.create()"},{"line_number":139,"context_line":"        return backup"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def _assert_backups_equal(self, reference, result):"},{"line_number":142,"context_line":"        reference \u003d vars(reference)"},{"line_number":143,"context_line":"        result \u003d vars(result)"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_c66bfd1c","line":141,"in_reply_to":"fa32b979_5c7fcbdf","updated":"2015-06-24 13:35:58.000000000","message":"If I change it to use assertDictMatch I\u0027ll have to map updated_at and all that start with _ to DONTCARE, which will basically take as many lines as this method.  XD","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"d384feb5018cc5c21984ddb51b13b60c9b6109b3","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        backup.create()"},{"line_number":139,"context_line":"        return backup"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def _assert_backups_equal(self, reference, result):"},{"line_number":142,"context_line":"        reference \u003d vars(reference)"},{"line_number":143,"context_line":"        result \u003d vars(result)"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_57c1d92d","line":141,"in_reply_to":"fa32b979_c66bfd1c","updated":"2015-06-24 14:15:46.000000000","message":"+1 to using assertDictMatch","commit_id":"d0d1a2b860d0e224210d9ee8ce2ca4d7b8332b70"}],"cinder/tests/unit/test_db_api.py":[{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7c47f120189372cf967a1513bb6df03a8656d0ac","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            len(obj1), len(obj2),"},{"line_number":75,"context_line":"            \"Keys mismatch: %s\" % str(set(obj1.keys()) ^ set(obj2.keys())))"},{"line_number":76,"context_line":"        for key, value in obj1.iteritems():"},{"line_number":77,"context_line":"            self.assertEqual(value, obj2[key], u\u0027on key %s\u0027 % key)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def _assertEqualListsOfObjects(self, objs1, objs2, ignored_keys\u003dNone):"},{"line_number":80,"context_line":"        obj_to_dict \u003d lambda o: self._dict_from_object(o, ignored_keys)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_334321b4","line":77,"updated":"2015-05-22 13:54:11.000000000","message":"six.text_type instead of `u`?","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"75dbb67e29ebd4f5dc5b8f8c8b207d6b948ada9f","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            len(obj1), len(obj2),"},{"line_number":75,"context_line":"            \"Keys mismatch: %s\" % str(set(obj1.keys()) ^ set(obj2.keys())))"},{"line_number":76,"context_line":"        for key, value in obj1.iteritems():"},{"line_number":77,"context_line":"            self.assertEqual(value, obj2[key], u\u0027on key %s\u0027 % key)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def _assertEqualListsOfObjects(self, objs1, objs2, ignored_keys\u003dNone):"},{"line_number":80,"context_line":"        obj_to_dict \u003d lambda o: self._dict_from_object(o, ignored_keys)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_15d15757","line":77,"in_reply_to":"7a016987_2671790c","updated":"2015-05-29 15:44:52.000000000","message":"`grep -r u\\\u0027 ./cinder/tests` found a lot of existing uses. I\u0027m OK with it now.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b696956a00680db146463d5f180e837e2b84db79","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            len(obj1), len(obj2),"},{"line_number":75,"context_line":"            \"Keys mismatch: %s\" % str(set(obj1.keys()) ^ set(obj2.keys())))"},{"line_number":76,"context_line":"        for key, value in obj1.iteritems():"},{"line_number":77,"context_line":"            self.assertEqual(value, obj2[key], u\u0027on key %s\u0027 % key)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def _assertEqualListsOfObjects(self, objs1, objs2, ignored_keys\u003dNone):"},{"line_number":80,"context_line":"        obj_to_dict \u003d lambda o: self._dict_from_object(o, ignored_keys)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_2671790c","line":77,"in_reply_to":"7a016987_334321b4","updated":"2015-05-26 11:40:27.000000000","message":"Why would we do that? This is already Python 2 and 3 compatible.","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7c47f120189372cf967a1513bb6df03a8656d0ac","unresolved":false,"context_lines":[{"line_number":1518,"context_line":"            backup_get \u003d db.backup_get(self.ctxt, backup[\u0027id\u0027])"},{"line_number":1519,"context_line":"            self._assertEqualObjects(backup, backup_get)"},{"line_number":1520,"context_line":""},{"line_number":1521,"context_line":"    def test_backup_get_deleted(self):"},{"line_number":1522,"context_line":"        for i in xrange(len(self.created)):"},{"line_number":1523,"context_line":"            if i \u0026 1:"},{"line_number":1524,"context_line":"                db.backup_destroy(self.ctxt, self.created[i][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_730bd97d","line":1521,"updated":"2015-05-22 13:54:11.000000000","message":"How about add test case for backup_get argument project_only?","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b696956a00680db146463d5f180e837e2b84db79","unresolved":false,"context_lines":[{"line_number":1518,"context_line":"            backup_get \u003d db.backup_get(self.ctxt, backup[\u0027id\u0027])"},{"line_number":1519,"context_line":"            self._assertEqualObjects(backup, backup_get)"},{"line_number":1520,"context_line":""},{"line_number":1521,"context_line":"    def test_backup_get_deleted(self):"},{"line_number":1522,"context_line":"        for i in xrange(len(self.created)):"},{"line_number":1523,"context_line":"            if i \u0026 1:"},{"line_number":1524,"context_line":"                db.backup_destroy(self.ctxt, self.created[i][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_835a464f","line":1521,"in_reply_to":"7a016987_730bd97d","updated":"2015-05-26 11:40:27.000000000","message":"Done","commit_id":"7c96aecc1f0d0341a760b7512cbaef14064428f3"}]}