Merge lp:~rvb/maas/cmd-line-bug-1190958 into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Do not print newline if Django.VERSION >= (1.5).

See https://docs.djangoproject.com/en/dev/howto/custom-management-commands/#management-commands-output.

[Jeroen T. Vermeulen (jtv)]

This seems unnecessarily fragile. Why not just strip the tuple and then add the newline unconditionally?

review: Needs Information

[Raphaël Badin (rvb)]

> This seems unnecessarily fragile. Why not just strip the tuple and then add
> the newline unconditionally?

I don't think this is fragile (detecting which version of Django is present seems pretty solid to me). Stripping the resulting string would work but my code has this advantage: when django pre 1.5 is long forgotten, we can simple get rid of the "if statement".

[Raphaël Badin (rvb)]

In other words, my strategy here is to make Django 1.5 the default expected behaviour and special case for Django.VERSION < 1.5.

[Gavin Panella (allenap)]

[1]

-        self.stdout.write('\n')
+        # In Django 1.5+, self.stdout.write() adds a newline character at
+        # the end of the message.

Please tell me that Django isn't meddling with the behaviour of
sys.stdout.write()? I assume you mean something else.

[2]

> In other words, my strategy here is to make Django 1.5 the default
> expected behaviour and special case for Django.VERSION < 1.5.

The advantage of Jeroen's idea is that we never need to care, even if
the behaviour changes back again in Django 1.6.

[Gavin Panella (allenap)]

Ignore [1]; sorry, missed the `self`.

[Gavin Panella (allenap)]

You might be able to ignore my [2] too; I'm not sure how Jeroen's idea works now. The tuple doesn't need stripping; the newline is added by Django in self.stdout.write().

[Raphaël Badin (rvb)]

> You might be able to ignore my [2] too; I'm not sure how Jeroen's idea works
> now. The tuple doesn't need stripping; the newline is added by Django in
> self.stdout.write().

You're right… I've done two other branches in the meantime and read Jeroen's comment too quickly…

[Jeroen T. Vermeulen (jtv)]

OK, that answers my question.

review: Approve

=== modified file 'src/maasserver/management/commands/apikey.py'
--- src/maasserver/management/commands/apikey.py	2013-05-16 06:32:28 +0000
+++ src/maasserver/management/commands/apikey.py	2013-06-14 12:18:26 +0000
@@ -19,6 +19,7 @@
     convert_string_to_tuple,
     convert_tuple_to_string,
     )
+import django
 from django.contrib.auth.models import User
 from django.core.management.base import (
     BaseCommand,
@@ -44,7 +45,10 @@
     def _print_token(self, token):
         """Write `token` to stdout in the standard format."""
         self.stdout.write(convert_tuple_to_string(get_creds_tuple(token)))
-        self.stdout.write('\n')
+        # In Django 1.5+, self.stdout.write() adds a newline character at
+        # the end of the message.
+        if django.VERSION < (1, 5):
+            self.stdout.write('\n')
 
     def _generate_token(self, user):
         _, token = user.get_profile().create_authorisation_token()