Merge lp:~jason-jasonamyers/maas/clearer-msg-for-api-204-returns into lp:maas/trunk

[Raphaël Badin]

Prints the word Success on receiving a http code of 20X from any API based command via maas-cli.

Prints the word Success on receiving a http code of 20X from any API based command via maas-cli

[Raphaël Badin (rvb)]

Thanks for the fix Jason!

Looks generally good but I think we could do a more general fix by printing "Success" for all the 20X responses.

In other words I suggest changing the check above the line you've changed ("response.status == 200") to deal with all 20X response codes instead of adding a new condition and modify the test that you've added accordingly (you should probably merge it with test_print_response_prints_textual_response_with_success_msg and randomly generate 20X response codes).

[jasonamyers (jason-jasonamyers)]

so would you like to print the line "Machine-readable output follows:\n"
even if there isn't going to be content like with a 204?

Thanks,
Jason

On Wed, Jan 7, 2015 at 3:49 AM, Raphaël Badin <email address hidden>
wrote:

> Thanks for the fix Jason!
>
> Looks generally good but I think we could do a more general fix by
> printing "Success" for all the 20X responses.
>
> In other words I suggest changing the check above the line you've changed
> ("response.status == 200") to deal with all 20X response codes instead of
> adding a new condition and modify the test that you've added accordingly
> (you should probably merge it with
> test_print_response_prints_textual_response_with_success_msg and randomly
> generate 20X response codes).
> --
>
> https://code.launchpad.net/~jason-jasonamyers/maas/clearer-msg-for-api-204-returns/+merge/245628
> You are the owner of
> lp:~jason-jasonamyers/maas/clearer-msg-for-api-204-returns.
>

[Raphaël Badin (rvb)]

> so would you like to print the line "Machine-readable output follows:\n"
> even if there isn't going to be content like with a 204?
>
> Thanks,
> Jason

Yeah, I think that's fine. Even a 200 response could have an empty response. As you pointed out, a 204 response is guaranteed to have an empty response but the textual output has to be clear and simple for human user. The information that the content is empty might be useful (even if the response is a 204).

[lp:~jason-jasonamyers/maas/clearer-msg-for-api-204-returns]

3441. By jasonamyers on 2015-01-07

Changing code to print a Success message on any HTTP status code of 20X, and updated test to use a random HTTP status code between 200 and 209

[Raphaël Badin (rvb)]

Thanks for all the changes. I think this is good to land now. Please just have a look at my remark about how you to check for success error codes. Once this is fixed you can mark the MP approved and the lander will take care of the rest.

review: Approve

[lp:~jason-jasonamyers/maas/clearer-msg-for-api-204-returns]

3442. By jasonamyers on 2015-01-08

Expanding the range from 20X to any HTTP 200 series status code.

=== modified file 'src/maascli/api.py'
--- src/maascli/api.py	2014-10-22 10:24:18 +0000
+++ src/maascli/api.py	2015-01-08 13:36:06 +0000
@@ -281,7 +281,7 @@
         If the response is textual, a trailing \n is appended.
         """
         if file.isatty():
-            if is_response_textual(response) and response.status == 200:
+            if is_response_textual(response) and response.status // 100 == 2:
                 file.write("Success.\n")
                 file.write("Machine-readable output follows:\n")
 

=== modified file 'src/maascli/tests/test_api.py'
--- src/maascli/tests/test_api.py	2014-10-22 10:24:18 +0000
+++ src/maascli/tests/test_api.py	2015-01-08 13:36:06 +0000
@@ -19,6 +19,7 @@
 import httplib
 import io
 import json
+import random
 import sys
 from textwrap import dedent
 
@@ -300,11 +301,12 @@
         self.assertEqual(response['content'], buf.getvalue())
 
     def test_print_response_prints_textual_response_with_success_msg(self):
-        # When the response has a status code of 200, and the response
+        # When the response has a status code of 2XX, and the response
         # body is textual print_response() will print a success message
         # to the TTY.
+        status_code = random.randrange(200, 300)
         response = httplib2.Response({
-            'status': httplib.OK,
+            'status': status_code,
             'content': "Lorem ipsum dolor sit amet.",
             'content-type': 'text/unicode',
             })