Code review comment for lp:~blake-rouse/maas/vlan-relay

Revision history for this message
Blake Rouse (blake-rouse) wrote :

I am not making that change in my branch. That is not even related to the
change I am making. File a bug if its causing an issue.

On Wed, Nov 30, 2016 at 4:14 PM, Mike Pontillo <email address hidden>
wrote:

>
>
> Diff comments:
>
> >
> > === added file 'src/maasserver/migrations/builtin/maasserver/0095_set_
> default_vlan_field.py'
> > --- src/maasserver/migrations/builtin/maasserver/0095_set_default_vlan_field.py
> 1970-01-01 00:00:00 +0000
> > +++ src/maasserver/migrations/builtin/maasserver/0095_set_default_vlan_field.py
> 2016-11-30 16:44:24 +0000
> > @@ -0,0 +1,24 @@
> > +# -*- coding: utf-8 -*-
> > +from __future__ import unicode_literals
> > +
> > +from django.db import (
> > + migrations,
> > + models,
> > +)
> > +import django.db.models.deletion
> > +import maasserver.models.subnet
> > +
> > +
> > +class Migration(migrations.Migration):
> > +
> > + dependencies = [
> > + ('maasserver', '0094_vlan_relay_vlan'),
> > + ]
> > +
> > + operations = [
> > + migrations.AlterField(
> > + model_name='subnet',
> > + name='vlan',
> > + field=models.ForeignKey(to='maasserver.VLAN',
> default=maasserver.models.subnet.get_default_vlan,
> on_delete=django.db.models.deletion.PROTECT),
>
> Ah, my mistake. But I think there might still be an issue here, just not
> exactly where I pointed it out.
>
> According to the Django docs, CASCADE is the default when no `on_delete`
> option is set. Wouldn't that cause severe unintended consequences in the
> case of the `relay_vlan` field? I think we should use `on_delete=SET_NULL`
> on the `relay_vlan` field.
>
> > + ),
> > + ]
>
>
> --
> https://code.launchpad.net/~blake-rouse/maas/vlan-relay/+merge/312165
> You are the owner of lp:~blake-rouse/maas/vlan-relay.
>

« Back to merge proposal