Code review comment for lp://staging/~johannes.erdfelt/glance/image-undelete

Revision history for this message
Johannes Erdfelt (johannes.erdfelt) wrote :

1) Can you be more detailed what extra testing you think is necessary?

2) I'll work on saving the previous status and restoring that value.

3) I agree that 'restore' is a better term. I'll make those changes shortly as well.

4) I can't say I'm 100% pleased with the /undelete member (as well as /restore). However, I think PUT is worse. PUT stores the request body at the resource specified. Restoring a deleted image doesn't update the body of the resource, just makes metadata changes.

Maybe /images/<id>/actions (ala nova) or /images/restore (taking image id as a POST variable) would be better choices?

There's also the alternative of making this more a client side operation, by allowing clients to retrieve the original status and then making the client use existing API entry points to restore the status. It would be trickier from an implementation standpoint since the existing code returns 404 for deleted (but not purged) images.

It could be that images that are pending delete still let metadata updates, but they aren't returned in list operations.

I think this one of those REST impedance mismatch issues so we'll need to be creative.

« Back to merge proposal