Merge lp:~rackspace-titan/glance/glance-user-confirm into lp:~hudson-openstack/glance/trunk

[Brian Waldon]

Expanding user confirmation default behavior.

[Vish Ishaya (vishvananda)]

37 + answer = raw_input("%s %s " % (prompt,prompt_default))

missed a space. Pep8 it?

Otherwise lgtm.

review: Approve

[lp:~rackspace-titan/glance/glance-user-confirm]

125. By Brian Waldon on 2011-04-22

docstring and exception handling

126. By Brian Waldon on 2011-04-22

pep8 fixes

[Brian Waldon (bcwaldon)]

> 37 + answer = raw_input("%s %s " % (prompt,prompt_default))
>
>
> missed a space. Pep8 it?
>
> Otherwise lgtm.

Got it. Also made one small functional change when an exception is caught.

[Brian Waldon (bcwaldon)]

> Got it. Also made one small functional change when an exception is caught.

Thinking about it more, it doesn't make sense to "return default" in an exception-case. Would "return not default" make sense? I don't want a delete to succeed when something legitimately wrong occurred.

[Vish Ishaya (vishvananda)]

no i don't like return not default. I actually think it might be a little excessive to try to catch an exception here and continue. Perhaps it is to handle the case when there is no tty? In that case i suppose return default would make sense. Otherwise I prefer just letting the exception bubble up.

[Dan Prince (dan-prince)]

You could let the exception trickle up? (not handle it).

I don't think returning 'not default' makes sense and could be dangerous.

[Brian Waldon (bcwaldon)]

Ok. I removed the except block. Should be good to go, now.

[lp:~rackspace-titan/glance/glance-user-confirm]

127. By Brian Waldon on 2011-04-22

removing excessive exception handling

[Dan Prince (dan-prince)]

Works great. Approve.

review: Approve

[Jay Pipes (jaypipes)]

yup, lgtm.

review: Approve

=== modified file 'bin/glance'
--- bin/glance	2011-04-20 23:58:10 +0000
+++ bin/glance	2011-04-22 15:16:29 +0000
@@ -317,7 +317,7 @@
         return FAILURE
 
     if not options.force and \
-       not user_confirm("Delete image %s?" % (image_id,)):
+       not user_confirm("Delete image %s?" % (image_id,), default=False):
         print 'Not deleting image %s' % (image_id,)
         return FAILURE
 
@@ -434,7 +434,8 @@
 %(prog)s clear [options]
 
 Deletes all images from a Glance server"""
-    if not options.force and not user_confirm("Delete all images?"):
+    if not options.force and \
+       not user_confirm("Delete all images?", default=False):
         print 'Not deleting any images'
         return FAILURE
 
@@ -546,12 +547,25 @@
     print COMMANDS[command].__doc__ % {'prog': os.path.basename(sys.argv[0])}
 
 
-def user_confirm(prompt):
-    try:
-        answer = raw_input("%s [Y/n] " % (prompt,))
+def user_confirm(prompt, default=False):
+    """Yes/No question dialog with user.
+
+    :param prompt: question/statement to present to user (string)
+    :param default: boolean value to return if empty string
+                    is received as response to prompt
+
+    """
+    if default:
+        prompt_default = "[Y/n]"
+    else:
+        prompt_default = "[y/N]"
+
+    answer = raw_input("%s %s " % (prompt, prompt_default))
+
+    if answer == "":
+        return default
+    else:
         return answer.lower() in ("yes", "y")
-    except Exception:
-        return False
 
 
 if __name__ == '__main__':