Code review comment for lp:~blake-rouse/maas/add-osystem-to-node-migration

Looks good, just a few comments.

[1]

Be sure to run 'make format',
src/maasserver/migrations/0077_add_osystem_to_node.py gets cleaned up a
bit by it.

[2]

=== added file 'src/maasserver/migrations/0077_add_osystem_to_node.py'
--- src/maasserver/migrations/0077_add_osystem_to_node.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/migrations/0077_add_osystem_to_node.py 2014-05-14 12:45:06 +0000

There is already another 0077 migration, so you'll have to merge and
regenerate this.

[3]

+ def forwards(self, orm):
+ # Adding field 'Node.osystem'
+ db.add_column(u'maasserver_node', 'osystem',
+ self.gf('django.db.models.fields.CharField')(default=u'', max_length=20, null=True, blank=True),
+ keep_default=False)

The max_length for Node's osystem field (20) doesn't match the max
length for BootImage's osystem field (255). If they're holding the
same set of values they should have the same types.

Also, Is there a set of osystem values a Node's osystem value should be
constrained to? Should a node always have an osystem that some BootImage
has, or is there some set they should both be constrained to?

[4]

There should be at least some basic test involving the osystem field.