Replace magic numbers with constants #3139

New Issue

adam · 2025-12-29T18:26:03+01:00

adam commented

2025-12-29 18:26:03 +01:00

Originally created by @lampwins on GitHub (Jan 9, 2020).

Originally assigned to: @jeremystretch on GitHub.

Proposed Changes

We have a number of instances in which we define "magic numbers" with little or no provided context. Particularly surrounding numeric min/max validators. We should properly define these in constants.py instead.

Justification

Defining these values as constants will allow for better descriptive use and also proper reuse throughout the codebase.

Originally created by @lampwins on GitHub (Jan 9, 2020). Originally assigned to: @jeremystretch on GitHub.  ### Proposed Changes We have a number of instances in which we define "magic numbers" with little or no provided context. Particularly surrounding numeric min/max validators. We should properly define these in constants.py instead.  ### Justification Defining these values as constants will allow for better descriptive use and also proper reuse throughout the codebase.

adam added the status: accepted type: housekeeping labels 2025-12-29 18:26:03 +01:00

adam closed this issue

2025-12-29 18:26:03 +01:00

adam commented

2025-12-29 18:26:04 +01:00

@jeremystretch commented on GitHub (Jan 9, 2020):

Just want to point out that the BGP ASN min/max values (#3876) weren't arbitrary; they're governed by the protocol. But yes, it makes sense to declare them as constants. What other examples did you have in mind?

Also: As part of the migration from integers to slug for ChoiceFields in v2.7, most of the old "magic numbers" have been removed and in some cases constants.py has been deleted (choice values were moved to choices.py within each app). I have no problem reinstating constants.py as needed though.

@jeremystretch commented on GitHub (Jan 9, 2020): Just want to point out that the BGP ASN min/max values (#3876) weren't arbitrary; they're governed by the protocol. But yes, it makes sense to declare them as constants. What other examples did you have in mind? Also: As part of the migration from integers to slug for ChoiceFields in v2.7, most of the old "magic numbers" have been removed and in some cases `constants.py` has been deleted (choice values were moved to `choices.py` within each app). I have no problem reinstating `constants.py` as needed though.

adam commented

2025-12-29 18:26:04 +01:00

@hSaria commented on GitHub (Jan 9, 2020):

I've found the following so far:

Interface's MTU
Service's port
VLANs

The MTU would benefit from the constants as the form widgets have a lower max value than the model validators.

@hSaria commented on GitHub (Jan 9, 2020): I've found the following so far: * Interface's MTU * Service's port * VLANs The MTU would benefit from the constants as the form widgets have a lower max value than the model validators.

adam commented

2025-12-29 18:26:05 +01:00

@lampwins commented on GitHub (Jan 9, 2020):

Really it boils down to, in most cases when we are defining a static value in the codebase, that value should be a variable. As @hSaria pointed out, there are several other instances of numeric validation and I will note that in those cases the values tend to be duplicated within both the model validation and the forms. There are some other things like in several instances when we define a color field, it has a length of 6.

I think this issue can be covered by a sweeping pass over the codebase and then, of course, going forward make it a development policy for any new values.

This is all about making the life of developers easier and making the codebase more maintainable as a whole, so to that end, we should still exercise good judgment in deciding if converting some value to a constant is actually more cumbersome than it is worth, to not do it then. For instance, several fields define a max_length of 100 but I would argue in that context it does not make sense to convert.

@lampwins commented on GitHub (Jan 9, 2020): Really it boils down to, in _most_ cases when we are defining a static value in the codebase, that value should be a variable. As @hSaria pointed out, there are several other instances of numeric validation and I will note that in those cases the values tend to be duplicated within both the model validation and the forms. There are some other things like in several instances when we define a color field, it has a length of 6. I think this issue can be covered by a sweeping pass over the codebase and then, of course, going forward make it a development policy for any new values. This is all about making the life of developers easier and making the codebase more maintainable as a whole, so to that end, we should still exercise good judgment in deciding if converting some value to a constant is actually more cumbersome than it is worth, to not do it then. For instance, several fields define a max_length of 100 but I would argue in that context it does not make sense to convert.

adam referenced this issue

2025-12-29 19:20:17 +01:00

Disable user password change if using REMOTE_AUTH #4770

adam referenced this issue

2025-12-29 22:22:38 +01:00

[PR #3667] [MERGED] Hide password change page when user is logged in using LDAP #12613

adam referenced this issue

2025-12-29 22:22:41 +01:00

[PR #3711] [MERGED] 822 bulk import of device components #12620

Sign in to join this conversation.