)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":10,"context_line":"snapshot when backing up cinder volumes, and to create"},{"line_number":11,"context_line":"a backup directly from the snapshots table."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Add snapshot id to backup table and backup detail"},{"line_number":14,"context_line":"Fix wrong template path in create_backup.html"},{"line_number":15,"context_line":"Add ChoiceField with snapshot ids belonging"},{"line_number":16,"context_line":"to the volume beeing backed up, or with the id of the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_daa6dbc1","line":13,"updated":"2019-06-27 13:34:54.000000000","message":"Could you itemize these? It makes easier to read the commit message.\n\n- Add snapshot id ...\n- Fix wrong template path ...\n- Add ChoiceField ....","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Add snapshot id to backup table and backup detail"},{"line_number":14,"context_line":"Fix wrong template path in create_backup.html"},{"line_number":15,"context_line":"Add ChoiceField with snapshot ids belonging"},{"line_number":16,"context_line":"to the volume beeing backed up, or with the id of the"},{"line_number":17,"context_line":"snapshot being backed up."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"See also, (though not implemented as described)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_5acd0b83","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"updated":"2019-06-27 13:34:54.000000000","message":"being","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":17,"context_line":"snapshot being backed up."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"See also, (though not implemented as described)"},{"line_number":20,"context_line":"https://blueprints.launchpad.net/horizon/+spec/volume-snapshot-backups"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I4b7707d95756501e0622460e3ddc4e3f7624f02e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_ba8a0718","line":20,"updated":"2019-06-27 13:34:54.000000000","message":"This commit needs a blueprint or a wishlist bug.\nHow is the blueprint you referred related to this commit?\nIf you work on this blueprint, we can assign you to the blueprint.","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b8f2df6f4ed1663fcf061ecbd869da23b5e25553","unresolved":false,"context_lines":[{"line_number":17,"context_line":"snapshot being backed up."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"See also, (though not implemented as described)"},{"line_number":20,"context_line":"https://blueprints.launchpad.net/horizon/+spec/volume-snapshot-backups"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I4b7707d95756501e0622460e3ddc4e3f7624f02e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_4d43bb6d","line":20,"in_reply_to":"9fb8cfa7_ba8a0718","updated":"2019-07-03 13:34:39.000000000","message":"This is still not addressed yet as of patch set 4.","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2550b6feea5414da5a04147a7c32f5029231fa13","unresolved":false,"context_lines":[{"line_number":20,"context_line":"When backing up from snapshots table the snapshot"},{"line_number":21,"context_line":"ChoiceField is preset with the choosen snapshot id"},{"line_number":22,"context_line":"as the only option."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Change-Id: I4b7707d95756501e0622460e3ddc4e3f7624f02e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7faddb67_e2acc68d","line":23,"updated":"2019-07-09 03:15:38.000000000","message":"Please add blueprint link here as recommended in the developer guide.\n\nhttps://docs.openstack.org/infra/manual/developers.html#committing-a-change","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"}],"openstack_dashboard/api/cinder.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":123,"context_line":"class VolumeBackup(BaseCinderAPIResourceWrapper):"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    _attrs \u003d [\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027snapshot_id\u0027, \u0027container\u0027, \u0027size\u0027,"},{"line_number":126,"context_line":"              \u0027created_at\u0027, \u0027status\u0027, \u0027created_at\u0027, \u0027volume_id\u0027,"},{"line_number":127,"context_line":"              \u0027availability_zone\u0027]"},{"line_number":128,"context_line":"    _volume \u003d None"},{"line_number":129,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_badd2715","line":126,"range":{"start_line":126,"start_character":14,"end_line":126,"end_character":26},"updated":"2019-06-27 13:34:54.000000000","message":"duplicated fields.....","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    _attrs \u003d [\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027snapshot_id\u0027, \u0027container\u0027, \u0027size\u0027,"},{"line_number":126,"context_line":"              \u0027created_at\u0027, \u0027status\u0027, \u0027created_at\u0027, \u0027volume_id\u0027,"},{"line_number":127,"context_line":"              \u0027availability_zone\u0027]"},{"line_number":128,"context_line":"    _volume \u003d None"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_1abf93aa","line":127,"updated":"2019-06-27 13:34:54.000000000","message":"Why are you changing the order of the fields?\nThis is just a question. There is no meaning in the order of the fields here, but you changed the order in addition to adding a new one.","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":632,"context_line":"                         name,"},{"line_number":633,"context_line":"                         description,"},{"line_number":634,"context_line":"                         force\u003dFalse,"},{"line_number":635,"context_line":"                         snapshot_id\u003d\u0027\u0027):"},{"line_number":636,"context_line":"    # need to ensure the container name is not an empty"},{"line_number":637,"context_line":"    # string, but pass None to get the container name"},{"line_number":638,"context_line":"    # generated correctly"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_3ad137fc","line":635,"range":{"start_line":635,"start_character":37,"end_line":635,"end_character":39},"updated":"2019-06-27 13:34:54.000000000","message":"Consider using None instead.","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":641,"context_line":"        container\u003dcontainer_name if container_name else None,"},{"line_number":642,"context_line":"        name\u003dname,"},{"line_number":643,"context_line":"        description\u003ddescription,"},{"line_number":644,"context_line":"        snapshot_id\u003dsnapshot_id if snapshot_id else None,"},{"line_number":645,"context_line":"        force\u003dforce)"},{"line_number":646,"context_line":"    return VolumeBackup(backup)"},{"line_number":647,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_ba068785","line":644,"updated":"2019-06-27 13:34:54.000000000","message":"If the default value is None as commented above, you can pass snapshot_id directly.","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"}],"openstack_dashboard/dashboards/project/backups/forms.py":[{"author":{"_account_id":27822,"name":"pengyuesheng","email":"pengyuesheng@gohighsec.com","username":"pengyuesheng"},"change_message_id":"d5fc1800f82ce352943f13adcc504030dd75116c","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"                except Exception:"},{"line_number":70,"context_line":"                    redirect \u003d reverse(\u0027horizon:project:volumes:index\u0027)"},{"line_number":71,"context_line":"                    if snap_id_val:"},{"line_number":72,"context_line":"                        redirect \u003d reverse(\u0027horizon:project:snaphots:index\u0027)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"                    exceptions.handle(request, _(\u0027Unable to fetch snapshots\u0027),"},{"line_number":75,"context_line":"                                      redirect\u003dredirect)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_1c7c5c70","line":72,"range":{"start_line":71,"start_character":20,"end_line":72,"end_character":76},"updated":"2019-06-28 04:10:07.000000000","message":"never be used","commit_id":"c45e0d988e70b54d469dfa0149f0fea932d39bfa"},{"author":{"_account_id":27822,"name":"pengyuesheng","email":"pengyuesheng@gohighsec.com","username":"pengyuesheng"},"change_message_id":"b3052310cc7c689f4cf8b115343cc694cd1bd2d8","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class CreateBackupForm(forms.SelfHandlingForm):"},{"line_number":33,"context_line":"    name \u003d forms.CharField(max_length\u003d255, label\u003d_(\"Backup Name\"))"},{"line_number":34,"context_line":"    description \u003d forms.CharField(widget\u003dforms.Textarea(attrs\u003d{\u0027rows\u0027: 4}),"},{"line_number":35,"context_line":"                                  label\u003d_(\"Description\"),"},{"line_number":36,"context_line":"                                  required\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_71a45109","line":33,"range":{"start_line":33,"start_character":4,"end_line":33,"end_character":8},"updated":"2019-07-05 03:12:41.000000000","message":"According to [1], the name is not required\n[1]https://docs.openstack.org/python-cinderclient/latest/cli/details.html#cinder-backup-create","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"de0c45b0c557fb55fb7e534bc2323535b7040f44","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"class CreateBackupForm(forms.SelfHandlingForm):"},{"line_number":33,"context_line":"    name \u003d forms.CharField(max_length\u003d255, label\u003d_(\"Backup Name\"))"},{"line_number":34,"context_line":"    description \u003d forms.CharField(widget\u003dforms.Textarea(attrs\u003d{\u0027rows\u0027: 4}),"},{"line_number":35,"context_line":"                                  label\u003d_(\"Description\"),"},{"line_number":36,"context_line":"                                  required\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_7ebf27fb","line":33,"range":{"start_line":33,"start_character":4,"end_line":33,"end_character":8},"in_reply_to":"7faddb67_71a45109","updated":"2019-07-08 07:27:06.000000000","message":"This comment is correct, but it is not related to this patch. If -1 is because of this, this is not a good idea.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        validators\u003d[containers_utils.no_slash_validator],"},{"line_number":41,"context_line":"        required\u003dFalse)"},{"line_number":42,"context_line":"    volume_id \u003d forms.CharField(widget\u003dforms.HiddenInput())"},{"line_number":43,"context_line":"    snapshot_id \u003d forms.ThemableChoiceField(label\u003d_(\"Select Snapshot\"),"},{"line_number":44,"context_line":"                                            required\u003dFalse)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def __init__(self, request, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_974c369f","line":43,"range":{"start_line":43,"start_character":53,"end_line":43,"end_character":68},"updated":"2019-07-09 03:13:56.000000000","message":"The title of the field should be \"Snapshot\" or \"Backup Snapshot\".\n\nIn addition, we need some help text here to describe what specifying a snapshot means. It would be like \"If a snapshot is specified here, a specified snapshot of the volume will be backuped.\"","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def __init__(self, request, *args, **kwargs):"},{"line_number":47,"context_line":"        super(CreateBackupForm, self).__init__(request, *args, **kwargs)"},{"line_number":48,"context_line":"        if \u0027initial\u0027 in kwargs and \u0027volume_id\u0027 in kwargs[\u0027initial\u0027]:"},{"line_number":49,"context_line":"            snap_id_val \u003d request.GET.get(\u0027snapshot_id\u0027, None)"},{"line_number":50,"context_line":"            if snap_id_val:"},{"line_number":51,"context_line":"                self.fields[\u0027snapshot_id\u0027].choices \u003d [(snap_id_val,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_576d5e02","line":48,"range":{"start_line":48,"start_character":11,"end_line":48,"end_character":30},"updated":"2019-07-09 03:13:56.000000000","message":"Looking at CreateBackupView, it seems that \u0027initial\u0027 is always populated. Why do you need to check \u0027initial\u0027 is contained in kwargs?\n\nSame question for volume_id.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    def __init__(self, request, *args, **kwargs):"},{"line_number":47,"context_line":"        super(CreateBackupForm, self).__init__(request, *args, **kwargs)"},{"line_number":48,"context_line":"        if \u0027initial\u0027 in kwargs and \u0027volume_id\u0027 in kwargs[\u0027initial\u0027]:"},{"line_number":49,"context_line":"            snap_id_val \u003d request.GET.get(\u0027snapshot_id\u0027, None)"},{"line_number":50,"context_line":"            if snap_id_val:"},{"line_number":51,"context_line":"                self.fields[\u0027snapshot_id\u0027].choices \u003d [(snap_id_val,"},{"line_number":52,"context_line":"                                                      snap_id_val)]"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_37ad4231","line":49,"range":{"start_line":49,"start_character":26,"end_line":49,"end_character":62},"updated":"2019-07-09 03:13:56.000000000","message":"From Django perspective, it is not recommended to extract \u0027snapshot_id\u0027 from \u0027request\u0027 variable directly. We suggest to define an entry in urls.py instead.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            snap_id_val \u003d request.GET.get(\u0027snapshot_id\u0027, None)"},{"line_number":50,"context_line":"            if snap_id_val:"},{"line_number":51,"context_line":"                self.fields[\u0027snapshot_id\u0027].choices \u003d [(snap_id_val,"},{"line_number":52,"context_line":"                                                      snap_id_val)]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"                self.fields[\u0027snapshot_id\u0027].initial \u003d snap_id_val"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_a2d32e38","line":52,"range":{"start_line":52,"start_character":54,"end_line":52,"end_character":65},"updated":"2019-07-09 03:13:56.000000000","message":"The indent level looks incorrect.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                self.fields[\u0027snapshot_id\u0027].choices \u003d [(snap_id_val,"},{"line_number":52,"context_line":"                                                      snap_id_val)]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"                self.fields[\u0027snapshot_id\u0027].initial \u003d snap_id_val"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            else:"},{"line_number":57,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_77497a89","line":54,"updated":"2019-07-09 03:13:56.000000000","message":"When I select \"Create Backup\" from the snapshot table, I see snapshot ID instead of snapshot name. I think \u0027snapshot name\u0027 is expected here.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                                                                search_opts\u003dsop)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"                    snapshots.sort(key\u003doperator.attrgetter(\u0027id\u0027, \u0027created_at\u0027))"},{"line_number":63,"context_line":"                    snapshotChoices \u003d [(\u0027\u0027, _(\"Select snapshot (optional)\"))]"},{"line_number":64,"context_line":"                    snapshotChoices.extend((snapshot.id, snapshot.name)"},{"line_number":65,"context_line":"                                           for snapshot in snapshots)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_2222de53","line":63,"updated":"2019-07-09 03:13:56.000000000","message":"If there is no snapshot for a target volume, \"No snapshot for a volume\" looks better as a label string.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"                    snapshots.sort(key\u003doperator.attrgetter(\u0027id\u0027, \u0027created_at\u0027))"},{"line_number":63,"context_line":"                    snapshotChoices \u003d [(\u0027\u0027, _(\"Select snapshot (optional)\"))]"},{"line_number":64,"context_line":"                    snapshotChoices.extend((snapshot.id, snapshot.name)"},{"line_number":65,"context_line":"                                           for snapshot in snapshots)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"                    self.fields[\u0027snapshot_id\u0027].choices \u003d snapshotChoices"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_0217a2ef","line":64,"range":{"start_line":64,"start_character":35,"end_line":64,"end_character":42},"updated":"2019-07-09 03:13:56.000000000","message":"Rather than extending a list one by one, it is better to create a list from \"snapshots\" first and then add a label at L.63 to the beginning.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":74,"context_line":"    def handle(self, request, data):"},{"line_number":75,"context_line":"        try:"},{"line_number":76,"context_line":"            volume \u003d api.cinder.volume_get(request, data[\u0027volume_id\u0027])"},{"line_number":77,"context_line":"            snapshot_id \u003d data[\u0027snapshot_id\u0027] if data[\u0027snapshot_id\u0027] else None"},{"line_number":78,"context_line":"            force \u003d False"},{"line_number":79,"context_line":"            if volume.status \u003d\u003d \u0027in-use\u0027:"},{"line_number":80,"context_line":"                force \u003d True"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_c2d66a28","line":77,"range":{"start_line":77,"start_character":26,"end_line":77,"end_character":78},"updated":"2019-07-09 03:13:56.000000000","message":"This can be simplified to:\n\n  data[\u0027snapshot_id\u0027] or None","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"}],"openstack_dashboard/dashboards/project/backups/tables.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cfcf55ba9698fd7efbb9d0523003567a741198d2","unresolved":false,"context_lines":[{"line_number":158,"context_line":"    volume_name \u003d BackupVolumeNameColumn(\"name\","},{"line_number":159,"context_line":"                                         verbose_name\u003d_(\"Volume Name\"),"},{"line_number":160,"context_line":"                                         link\u003d\"horizon:project:volumes:detail\")"},{"line_number":161,"context_line":"    snapshot_id \u003d SnapshotIdColumn(\"snapshot_id\","},{"line_number":162,"context_line":"                                   verbose_name\u003d_(\"Snapshot ID\"),"},{"line_number":163,"context_line":"                                   link\u003d\"horizon:project:snapshots:detail\")"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_18a316a0","line":161,"range":{"start_line":161,"start_character":4,"end_line":161,"end_character":16},"updated":"2019-06-27 13:34:54.000000000","message":"We don\u0027t show ID in almost all cases. Name should be used instead. The only exception is a case where \u0027name\u0027 is not provided as an attribute. My recommendation is to show \u0027name\u0027.","commit_id":"b2401552dafcb6f1563bcbc34a705707dfc01b28"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return safestring.mark_safe(snapshot_name)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def get_link_url(self, backup):"},{"line_number":60,"context_line":"        if backup.snapshot and backup.snapshot_id:"},{"line_number":61,"context_line":"            return reverse(\u0027horizon:project:snapshots:detail\u0027,"},{"line_number":62,"context_line":"                           args\u003d(backup.snapshot_id,))"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_02c5625b","line":60,"range":{"start_line":60,"start_character":11,"end_line":60,"end_character":49},"updated":"2019-07-09 03:13:56.000000000","message":"backup.snapshot is populated in views.py and None is populated when snapshot_id is empty, so you don\u0027t need to check backup.snapshot_id here.\n\nI am not sure you want to show snapshot_id when backup.snapshot is None (i.e., when a corresponding snapshot information cannot be retrieved). IMHO a backup is still associated to a snapshot so it is better to provide some information instead of showing \u0027-\u0027 (which looks confusing to me).","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"cbf8f46a9dd2fcd4c217670923c68a70c84be6c3","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return safestring.mark_safe(snapshot_name)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def get_link_url(self, backup):"},{"line_number":60,"context_line":"        if backup.snapshot and backup.snapshot_id:"},{"line_number":61,"context_line":"            return reverse(\u0027horizon:project:snapshots:detail\u0027,"},{"line_number":62,"context_line":"                           args\u003d(backup.snapshot_id,))"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_c6f3b009","line":60,"range":{"start_line":60,"start_character":11,"end_line":60,"end_character":49},"in_reply_to":"7faddb67_02c5625b","updated":"2019-07-11 06:47:00.000000000","message":"(copying a comment from the author to make it easy to follow the discussion)\n\nI\u0027m setting snapshot_name to _(\"Unknown\") on line 54 in openstack_dashboard/dashboards/project/backups/tables.py, in case of a backup.snapshot_id and when no snapshot with that id could be found. So the dash \"-\" only appears if there was no snapshot associated when the backup was done.. \nSo I\u0027m not quite sure what you mean here..","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"f5e85d9e33667ab6381616597d402b80539c6e8c","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return safestring.mark_safe(snapshot_name)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def get_link_url(self, backup):"},{"line_number":60,"context_line":"        if backup.snapshot and backup.snapshot_id:"},{"line_number":61,"context_line":"            return reverse(\u0027horizon:project:snapshots:detail\u0027,"},{"line_number":62,"context_line":"                           args\u003d(backup.snapshot_id,))"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_26cba41e","line":60,"range":{"start_line":60,"start_character":11,"end_line":60,"end_character":49},"in_reply_to":"7faddb67_c6f3b009","updated":"2019-07-11 07:49:07.000000000","message":"What I would like to mention is \"if backup.snapshot\" is enough here. You would like to show a link to a snapshot detail only when a corresponding snapshot is found. If a snapshot is found, backup.snapshot is always populated.\n\nRegarding the second point, you are correct. \"Unknown\" is shown. Let\u0027s keep the current one.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def get_link_url(self, backup):"},{"line_number":60,"context_line":"        if backup.snapshot and backup.snapshot_id:"},{"line_number":61,"context_line":"            return reverse(\u0027horizon:project:snapshots:detail\u0027,"},{"line_number":62,"context_line":"                           args\u003d(backup.snapshot_id,))"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"class DeleteBackup(tables.DeleteAction):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_a2a1eeab","line":62,"range":{"start_line":62,"start_character":33,"end_line":62,"end_character":51},"updated":"2019-07-09 03:13:56.000000000","message":"If you skip snapshot_id check at L.60 per my comment, \"backup.snapshot.id\" would be better here.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"02b5d28c78d76b3ca986b0e1358c9f5d3d2eab67","unresolved":false,"context_lines":[{"line_number":123,"context_line":"            pass"},{"line_number":124,"context_line":"        try:"},{"line_number":125,"context_line":"            backup.snapshot \u003d cinder.volume_snapshot_get(request,"},{"line_number":126,"context_line":"                                                         backup.snapshot_id)"},{"line_number":127,"context_line":"        except Exception:"},{"line_number":128,"context_line":"            pass"},{"line_number":129,"context_line":"        return backup"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_82b952ea","line":126,"updated":"2019-07-09 03:13:56.000000000","message":"Could you avoid calling volume_snapshot_get when snapshot_id is None?","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"}],"openstack_dashboard/dashboards/project/backups/templates/backups/_detail_overview.html":[{"author":{"_account_id":27822,"name":"pengyuesheng","email":"pengyuesheng@gohighsec.com","username":"pengyuesheng"},"change_message_id":"d5fc1800f82ce352943f13adcc504030dd75116c","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \u003cdt\u003e{% trans \"Snapshot\" %}\u003c/dt\u003e"},{"line_number":25,"context_line":"    \u003cdd\u003e"},{"line_number":26,"context_line":"     \u003ca href\u003d\"{% url \u0027horizon:project:snapshots:detail\u0027 backup.snapshot_id %}\"\u003e"},{"line_number":27,"context_line":"\t{{ snapshot.name }}"},{"line_number":28,"context_line":"     \u003c/a\u003e"},{"line_number":29,"context_line":"    {% endif %}"},{"line_number":30,"context_line":"\u003c/dd\u003e"}],"source_content_type":"text/html","patch_set":2,"id":"9fb8cfa7_5cebb411","line":27,"range":{"start_line":27,"start_character":0,"end_line":27,"end_character":1},"updated":"2019-06-28 04:10:07.000000000","message":"Should use a space instead of a tab","commit_id":"c45e0d988e70b54d469dfa0149f0fea932d39bfa"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"afd7106c485fd99ad8295c20aa01c74d0163ec0b","unresolved":false,"context_lines":[{"line_number":20,"context_line":"      \u003c/a\u003e"},{"line_number":21,"context_line":"    \u003c/dd\u003e"},{"line_number":22,"context_line":"    {% endif %}"},{"line_number":23,"context_line":"    {% if  snapshot and backup.snapshot_id %}"},{"line_number":24,"context_line":"    \u003cdt\u003e{% trans \"Snapshot\" %}\u003c/dt\u003e"},{"line_number":25,"context_line":"    \u003cdd\u003e"},{"line_number":26,"context_line":"     \u003ca href\u003d\"{% url \u0027horizon:project:snapshots:detail\u0027 backup.snapshot_id %}\"\u003e"}],"source_content_type":"text/html","patch_set":4,"id":"7faddb67_a6ff9432","line":23,"range":{"start_line":23,"start_character":4,"end_line":23,"end_character":45},"updated":"2019-07-11 07:54:59.000000000","message":"This means \"Snapshot\" field is not shown if a corresponding snapshot is not found. I think it is better to show \"Snapshot\" field even when a corresponding snapshot has been deleted. How about just showing a snapshot ID without a link?\n\nIt would be more consistent with the backup table.","commit_id":"bb0cb8688e3cd0e5caad91ae4e6b20482efc4ffa"}],"openstack_dashboard/dashboards/project/backups/tests.py":[{"author":{"_account_id":27822,"name":"pengyuesheng","email":"pengyuesheng@gohighsec.com","username":"pengyuesheng"},"change_message_id":"d5fc1800f82ce352943f13adcc504030dd75116c","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class VolumeBackupsViewTests(test.TestCase):"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @test.create_mocks({api.cinder: (\u0027volume_list\u0027, \u0027volume_snapshot_list\u0027,"},{"line_number":31,"context_line":"                                     \u0027volume_backup_list_paged\u0027,"},{"line_number":32,"context_line":"                                     \u0027volume_snapshot_get\u0027)})"},{"line_number":33,"context_line":"    def _test_backups_index_paginated(self, marker, sort_dir, backups, url,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_3cf4c0f0","line":30,"range":{"start_line":30,"start_character":53,"end_line":30,"end_character":73},"updated":"2019-06-28 04:10:07.000000000","message":"not used","commit_id":"c45e0d988e70b54d469dfa0149f0fea932d39bfa"},{"author":{"_account_id":27822,"name":"pengyuesheng","email":"pengyuesheng@gohighsec.com","username":"pengyuesheng"},"change_message_id":"d5fc1800f82ce352943f13adcc504030dd75116c","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @test.create_mocks({api.cinder: (\u0027volume_list\u0027, \u0027volume_snapshot_list\u0027,"},{"line_number":31,"context_line":"                                     \u0027volume_backup_list_paged\u0027,"},{"line_number":32,"context_line":"                                     \u0027volume_snapshot_get\u0027)})"},{"line_number":33,"context_line":"    def _test_backups_index_paginated(self, marker, sort_dir, backups, url,"},{"line_number":34,"context_line":"                                      has_more, has_prev):"},{"line_number":35,"context_line":"        self.mock_volume_backup_list_paged.return_value \u003d [backups,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_bc6670ba","line":32,"range":{"start_line":32,"start_character":38,"end_line":32,"end_character":57},"updated":"2019-06-28 04:10:07.000000000","message":"not used","commit_id":"c45e0d988e70b54d469dfa0149f0fea932d39bfa"},{"author":{"_account_id":27822,"name":"pengyuesheng","email":"pengyuesheng@gohighsec.com","username":"pengyuesheng"},"change_message_id":"c3424441ff75e1047d9373f7ff9bb3b85a8a59cc","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class VolumeBackupsViewTests(test.TestCase):"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @test.create_mocks({api.cinder: (\u0027volume_list\u0027, \u0027volume_snapshot_list\u0027,"},{"line_number":31,"context_line":"                                     \u0027volume_backup_list_paged\u0027)})"},{"line_number":32,"context_line":"    def _test_backups_index_paginated(self, marker, sort_dir, backups, url,"},{"line_number":33,"context_line":"                                      has_more, has_prev):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_cf903146","line":30,"range":{"start_line":30,"start_character":53,"end_line":30,"end_character":73},"updated":"2019-07-01 03:29:55.000000000","message":"I saw that you introduced this interface, why didn\u0027t you use it below?","commit_id":"28ef8167844829bb0744920bb51eb0a87b62f1be"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"f293d7109d83a6a5c8a8c4e2b6f8f89a331d02e6","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class VolumeBackupsViewTests(test.TestCase):"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @test.create_mocks({api.cinder: (\u0027volume_list\u0027, \u0027volume_snapshot_list\u0027,"},{"line_number":31,"context_line":"                                     \u0027volume_backup_list_paged\u0027)})"},{"line_number":32,"context_line":"    def _test_backups_index_paginated(self, marker, sort_dir, backups, url,"},{"line_number":33,"context_line":"                                      has_more, has_prev):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_425406fe","line":30,"range":{"start_line":30,"start_character":53,"end_line":30,"end_character":73},"in_reply_to":"9fb8cfa7_cf903146","updated":"2019-07-02 02:58:06.000000000","message":"+1 to pengyuesheng. All mocked methods should set their return values explicitly and be verified in the test code.","commit_id":"28ef8167844829bb0744920bb51eb0a87b62f1be"}]}
