sts-sponsors team mailing list archive

[Björn Tillenius <mp+438041@xxxxxxxxxxxxxxxxxx>]

Review: Needs Information



Diff comments:

> diff --git a/src/maasserver/fields.py b/src/maasserver/fields.py
> index 276d4d4..7c79687 100644
> --- a/src/maasserver/fields.py
> +++ b/src/maasserver/fields.py
> @@ -50,19 +72,17 @@ class MACAddressFormField(forms.CharField):
>          )
>  
>  
> -class MACAddressField(Field):
> +class MACAddressField(TextField):
>      """Model field type: MAC address."""
>  
>      description = "MAC address"
>  
>      default_validators = [MAC_VALIDATOR]
>  
> -    def db_type(self, *args, **kwargs):
> -        return "macaddr"
> -
> -    def get_prep_value(self, value):
> -        # Convert empty string to None.
> -        return super().get_prep_value(value) or None
> +    def to_python(self, value):
> +        if not value:
> +            return value
> +        return normalise_macaddress(value)

Could we avoid doing this in the DB field? It makes sense when dealing with user input, but for code that writes to the database we should be more strict and require it to be in the right format.

I'd like us to avoid writing one value to the database and then when you read it back, you get something else.

>  
>  
>  class XMLField(Field):
> diff --git a/src/maasserver/tests/test_fields.py b/src/maasserver/tests/test_fields.py
> index 24014da..99cc72d 100644
> --- a/src/maasserver/tests/test_fields.py
> +++ b/src/maasserver/tests/test_fields.py
> @@ -64,7 +64,7 @@ class TestModelNameValidator(MAASServerTestCase):
>  class TestMACAddressField(MAASServerTestCase):
>      def test_mac_address_is_stored_normalized_and_loaded(self):
>          interface = factory.make_Interface(
> -            INTERFACE_TYPE.PHYSICAL, mac_address=" AA-bb-CC-dd-EE-Ff "
> +            INTERFACE_TYPE.PHYSICAL, mac_address="AA-bb-CC-dd-EE-Ff"

For example, for code like this we should pass in the mac using the right format, and not rely on implicit conversion.

>          )
>          loaded_mac = Interface.objects.get(id=interface.id)
>          self.assertEqual("aa:bb:cc:dd:ee:ff", loaded_mac.mac_address)


-- 
https://code.launchpad.net/~ack/maas/+git/maas/+merge/438041
Your team MAAS Committers is subscribed to branch maas:master.