Merge lp:~jtv/maas/register-mac-type into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Register MAC type with psycopg during program startup, not while importing modules.

It's not appropriate for module-level code to attempt to connect the database. It's an uncontrolled environment, and lint checkers may exercise the code by accident.

I did not try this out with a packaged install (although the tests should ensure that that will work) because we don't have a package with the requisite "array" extension yet.

Jeroen

[Julian Edwards (julian-edwards)]

 review: approve

Looks good.

review: Approve

[MAAS Lander (maas-lander)]

No proposals found for merge of lp:~jtv/maas/router-macs into lp:maas.

=== modified file 'src/maasserver/fields.py'
--- src/maasserver/fields.py	2013-08-23 07:03:07 +0000
+++ src/maasserver/fields.py	2013-08-23 07:03:07 +0000
@@ -14,6 +14,7 @@
     "MAC",
     "MACAddressField",
     "MACAddressFormField",
+    "register_mac_type",
     ]
 
 from copy import deepcopy
@@ -25,7 +26,6 @@
 
 from django.core.exceptions import ValidationError
 from django.core.validators import RegexValidator
-from django.db import connection
 from django.db.models import (
     Field,
     SubfieldBase,
@@ -234,10 +234,6 @@
         (oid, ), b"macaddr", mac_caster))
 
 
-# TODO bug=1215446: Don't do this at the module level!
-register_mac_type(connection.cursor())
-
-
 class JSONObjectField(Field):
     """A field that will store any jsonizable python object."""
 

=== modified file 'src/maasserver/start_up.py'
--- src/maasserver/start_up.py	2013-05-28 16:06:35 +0000
+++ src/maasserver/start_up.py	2013-08-23 07:03:07 +0000
@@ -17,6 +17,7 @@
 
 from textwrap import dedent
 
+from django.db import connection
 from lockfile import FileLock
 from maasserver.components import (
     get_persistent_error,
@@ -24,6 +25,7 @@
     )
 from maasserver.dns import write_full_dns_config
 from maasserver.enum import COMPONENT
+from maasserver.fields import register_mac_type
 from maasserver.maasavahi import setup_maas_avahi_service
 from maasserver.models import (
     BootImage,
@@ -79,6 +81,9 @@
 
 def inner_start_up():
     """Startup jobs that must run serialized w.r.t. other starting servers."""
+    # Register our MAC data type with psycopg.
+    register_mac_type(connection.cursor())
+
     # Publish the MAAS server existence over Avahi.
     setup_maas_avahi_service()
 

=== modified file 'src/maasserver/testing/testcase.py'
--- src/maasserver/testing/testcase.py	2013-06-14 16:03:35 +0000
+++ src/maasserver/testing/testcase.py	2013-08-23 07:03:07 +0000
@@ -24,8 +24,10 @@
 import django
 from django.core.cache import cache as django_cache
 from django.core.urlresolvers import reverse
+from django.db import connection
 from django.test.client import encode_multipart
 from fixtures import Fixture
+from maasserver.fields import register_mac_type
 from maasserver.testing.factory import factory
 from maastesting.celery import CeleryFixture
 import maastesting.djangotestcase
@@ -43,6 +45,10 @@
 class TestCase(maastesting.djangotestcase.DjangoTestCase):
     """:class:`TestCase` variant with the basics for maasserver testing."""
 
+    @classmethod
+    def setUpClass(cls):
+        register_mac_type(connection.cursor())
+
     def setUp(self):
         super(TestCase, self).setUp()
         self.useFixture(WorkerCacheFixture())

=== modified file 'src/maasserver/tests/test_fields.py'
--- src/maasserver/tests/test_fields.py	2013-08-23 07:03:07 +0000
+++ src/maasserver/tests/test_fields.py	2013-08-23 07:03:07 +0000
@@ -13,10 +13,14 @@
 __all__ = []
 
 from django.core.exceptions import ValidationError
-from django.db import DatabaseError
+from django.db import (
+    connection,
+    DatabaseError,
+    )
 from maasserver.fields import (
     MAC,
     NodeGroupFormField,
+    register_mac_type,
     validate_mac,
     )
 from maasserver.models import (
@@ -160,6 +164,12 @@
             set([MAC(addr), MAC(addr), MAC(MAC(addr)), addr]),
             [addr])
 
+    def test_register_mac_type_is_idempotent(self):
+        register_mac_type(connection.cursor())
+        register_mac_type(connection.cursor())
+        # The test is that we get here without crashing.
+        pass
+
 
 class TestMACAddressField(TestCase):