launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-arch into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-arch/+merge/94799

This branch adds a new field to the Node model: architecture.  I considered making it a IntegerField but I thought it would be clearer to have a CharField with the real name of the architecture instead.

Note that this field is optional (i.e. can be None) because I haven't done anything to add this field to the UI yet.

Drive-by fix:
- factor the list of Node fields to be published by piston in NODE_FIELDS.
-- 
https://code.launchpad.net/~rvb/maas/maas-arch/+merge/94799
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-arch into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-02-25 12:40:38 +0000
+++ src/maasserver/api.py	2012-02-27 16:56:18 +0000
@@ -193,12 +193,17 @@
     return cls
 
 
+NODE_FIELDS = (
+    'system_id', 'hostname', ('macaddress_set', ('mac_address',)),
+    'architecture')
+
+
 @api_operations
 class NodeHandler(BaseHandler):
     """Manage individual Nodes."""
     allowed_methods = ('GET', 'DELETE', 'POST', 'PUT')
     model = Node
-    fields = ('system_id', 'hostname', ('macaddress_set', ('mac_address',)))
+    fields = NODE_FIELDS
 
     def read(self, request, system_id):
         """Read a specific Node."""
@@ -267,7 +272,7 @@
 class AnonNodesHandler(AnonymousBaseHandler):
     """Create Nodes."""
     allowed_methods = ('POST',)
-    fields = ('system_id', 'hostname', ('macaddress_set', ('mac_address',)))
+    fields = NODE_FIELDS
 
     @api_exported('new', 'POST')
     def new(self, request):

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-02-27 12:36:55 +0000
+++ src/maasserver/forms.py	2012-02-27 16:56:18 +0000
@@ -26,6 +26,7 @@
     )
 from maasserver.fields import MACAddressFormField
 from maasserver.models import (
+    ARCHITECTURE_CHOICES,
     Config,
     MACAddress,
     Node,
@@ -34,6 +35,11 @@
     )
 
 
+INVALID_ARCHITECTURE_MESSAGE = (
+    "%(value)s is not a valid architecture. " +
+    "It should be one of: %s." % ", ".join(dict(ARCHITECTURE_CHOICES).keys()))
+
+
 class NodeForm(ModelForm):
     system_id = forms.CharField(
         widget=forms.TextInput(attrs={'readonly': 'readonly'}),
@@ -41,10 +47,15 @@
     after_commissioning_action = forms.TypedChoiceField(
         choices=NODE_AFTER_COMMISSIONING_ACTION_CHOICES, required=False,
         empty_value=NODE_AFTER_COMMISSIONING_ACTION.DEFAULT)
+    architecture = forms.ChoiceField(
+        choices=ARCHITECTURE_CHOICES, required=False,
+        error_messages={'invalid_choice': INVALID_ARCHITECTURE_MESSAGE})
 
     class Meta:
         model = Node
-        fields = ('hostname', 'system_id', 'after_commissioning_action')
+        fields = (
+            'hostname', 'system_id', 'after_commissioning_action',
+            'architecture')
 
 
 class MACAddressForm(ModelForm):

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-02-24 17:37:01 +0000
+++ src/maasserver/models.py	2012-02-27 16:56:18 +0000
@@ -158,6 +158,19 @@
     NODE_AFTER_COMMISSIONING_ACTION_CHOICES)
 
 
+# List of supported architectures.
+class ARCHITECTURE:
+    i386 = 'i386'
+    amd64 = 'amd64'
+
+
+# Architecture names.
+ARCHITECTURE_CHOICES = (
+    (ARCHITECTURE.i386, "i386"),
+    (ARCHITECTURE.amd64, "amd64"),
+)
+
+
 class NodeManager(models.Manager):
     """A utility to manage the collection of Nodes."""
 
@@ -330,6 +343,9 @@
         choices=NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
         default=NODE_AFTER_COMMISSIONING_ACTION.DEFAULT)
 
+    architecture = models.CharField(max_length=10,
+        choices=ARCHITECTURE_CHOICES, blank=True)
+
     objects = NodeManager()
 
     def __unicode__(self):

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-02-27 12:25:34 +0000
+++ src/maasserver/tests/test_api.py	2012-02-27 16:56:18 +0000
@@ -18,6 +18,7 @@
 
 from django.conf import settings
 from maasserver.models import (
+    ARCHITECTURE,
     MACAddress,
     Node,
     NODE_STATUS,
@@ -53,6 +54,7 @@
             {
                 'op': 'new',
                 'hostname': 'diane',
+                'architecture': 'amd64',
                 'after_commissioning_action': '2',
                 'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'],
             })
@@ -64,6 +66,7 @@
         self.assertNotEqual(0, len(parsed_result.get('system_id')))
         [diane] = Node.objects.filter(hostname='diane')
         self.assertEqual(2, diane.after_commissioning_action)
+        self.assertEqual(ARCHITECTURE.amd64, diane.architecture)
 
     def test_POST_new_associates_mac_addresses(self):
         # The API allows a Node to be created and associated with MAC
@@ -92,7 +95,8 @@
             })
         parsed_result = json.loads(response.content)
         self.assertItemsEqual(
-            ['hostname', 'system_id', 'macaddress_set'], parsed_result.keys())
+            ['hostname', 'system_id', 'macaddress_set', 'architecture'],
+            parsed_result.keys())
 
     def test_POST_fails_without_operation(self):
         # If there is no operation ('op=operation_name') specified in the
@@ -142,6 +146,23 @@
             ["One or more MAC Addresses is invalid."],
             parsed_result['mac_addresses'])
 
+    def test_POST_invalid_architecture_returns_bad_request(self):
+        # If the architecture name provided to create a node is not a valid
+        # architecture name, a 'Bad request' response is returned.
+        response = self.client.post(
+            '/api/nodes/',
+            {
+                'op': 'new',
+                'hostname': 'diane',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+                'architecture': 'invalid-architecture',
+            })
+        parsed_result = json.loads(response.content)
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn('application/json', response['Content-Type'])
+        self.assertItemsEqual(['architecture'], parsed_result)
+
 
 class NodeAnonAPITest(APIv10TestMixin, TestCase):