Skip to content

Conversation

@huntabyte
Copy link
Contributor

Fixes: #7702

Implemented the ability for users to override the default power feed values via user-configurable preferences.

I ran into a bit of a fork in the road when adding this functionality. Initially, I was replacing the model defaults with config items, for example:

class PowerFeed(NetBoxModel, PathEndpoint, LinkTermination):
    """
    An electrical circuit delivered from a PowerPanel.
    """
    voltage = models.SmallIntegerField(
        default=ConfigItem('POWERFEED_DEFAULT_VOLTAGE')
        validators=[ExclusionValidator([0])]
    )
    amperage = models.PositiveSmallIntegerField(
        validators=[MinValueValidator(1)],
        default=ConfigItem('POWERFEED_DEFAULT_AMPERAGE')
    )
    max_utilization = models.PositiveSmallIntegerField(
        validators=[MinValueValidator(1), MaxValueValidator(100)],
        default=ConfigItem('POWERFEED_DEFAULT_MAX_UTILIZATION')
        help_text="Maximum permissible draw (percentage)"

Django's documentation regarding migrations does state the following:

Django never sets database defaults and always applies them in the Django ORM code.

But after investing the rest of the codebase, I didn't see user-defined values as defaults for any of the other models, so I assumed the standard is to let the view handle it:

class PowerFeedCreateView(generic.ObjectEditView):
    queryset = PowerFeed.objects.all()
    form = forms.PowerFeedForm

    def alter_object(self, obj, request, args, kwargs):
        obj.voltage = ConfigItem('POWERFEED_DEFAULT_VOLTAGE')
        obj.amperage = ConfigItem('POWERFEED_DEFAULT_AMPERAGE')
        obj.max_utilization = ConfigItem('POWERFEED_DEFAULT_MAX_UTILIZATION')
        return obj

However, if the maintainers decide otherwise or see no reason not to allow users to define the model defaults, I can make that adjustment.

@jeremystretch
Copy link
Member

But after investing the rest of the codebase, I didn't see user-defined values as defaults for any of the other models

That's correct; we don't have an established pattern for feeding customizable defaults to model fields.

I assumed the standard is to let the view handle it

I see three places we could do it:

  • Model field
  • UI view (object instantiation)
  • Form field

Let's move these to the model fields, for two reasons:

  1. Ensures consistent application of the configured defaults for both UI- and API-created objects
  2. Avoids needing to creating a redundant view

I just tested this locally and it seems to work as expected. The change does trigger the creation of a new database migration, but as you noted above this doesn't actually impact the schema, so we could "fake" these changes in a prior migration file to avoid the noise. (I can handle that part.)

@huntabyte do you want to make these changes? If not, I can tackle them. Thanks!

@huntabyte
Copy link
Contributor Author

Yes, I will move these over to the model fields.

@jeremystretch jeremystretch merged commit f7de261 into netbox-community:develop Jun 23, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow configuration of Netbox powerfeed defaults

2 participants