Replace numeric constants with slug values

[jeremystretch]

Proposed Changes

Replace the numeric values used for many fields with human-friendly slug values. For example:

RACK_STATUS_RESERVED = 0
RACK_STATUS_AVAILABLE = 1
RACK_STATUS_PLANNED = 2
RACK_STATUS_ACTIVE = 3
RACK_STATUS_DEPRECATED = 4
RACK_STATUS_CHOICES = [
    [RACK_STATUS_ACTIVE, 'Active'],
    [RACK_STATUS_PLANNED, 'Planned'],
    [RACK_STATUS_RESERVED, 'Reserved'],
    [RACK_STATUS_AVAILABLE, 'Available'],
    [RACK_STATUS_DEPRECATED, 'Deprecated'],
]

would become

RACK_STATUS_RESERVED = 'active'
RACK_STATUS_AVAILABLE = 'planned'
RACK_STATUS_PLANNED = 'reserved'
RACK_STATUS_ACTIVE = 'available'
RACK_STATUS_DEPRECATED = 'deprecated'
RACK_STATUS_CHOICES = [
    [RACK_STATUS_ACTIVE, 'Active'],
    [RACK_STATUS_PLANNED, 'Planned'],
    [RACK_STATUS_RESERVED, 'Reserved'],
    [RACK_STATUS_AVAILABLE, 'Available'],
    [RACK_STATUS_DEPRECATED, 'Deprecated'],
]

Justification

Employing human-friendly slug values make consuming the REST API more convenient. It also allows more human-friendly representations of the pertinent field values in other formats, such as YAML (see #451).

[candlerb]

Would this also change the database column types to enum?

[jeremystretch]

I don't think Django has built-in support for enumeration types, and adding it probably wouldn't get us much. It would also mean incurring a new migration each time a field had its set of valid choices modified.

[candlerb]

In that case, surely the numeric values for RACK_STATUS_RESERVED, RACK_STATUS_AVAILABLE etc still need to be preserved in the Python code?

You could instead have RACK_STATUS_DISPLAY_CHOICES and RACK_STATUS_API_CHOICES ?

[jeremystretch]

The point of the FR is to replace the numeric values with strings entirely. This will occur within a migration and be completely transparent to the user.

[candlerb]

So the database field will be a varchar, not an enum? Got it.

[jeremystretch]

Yep. I should point out that there will almost certainly be a transition period where the API will display and accept both the numeric and string values.

[jeremystretch]

Something else occurs to me: By switching from integers to strings we'll lose the ability to order interfaces logically by type (ordering by type would be alphabetical using strings). I don't think this much of a concern but I wanted to put it out there.

[DanSheps]

Could we define a sort order or a sort function for those perhaps?

I don't think the UI has any place to sort by them right now though, so I don't know if it is worth it until there is plans to actually have sorting for them.

[jeremystretch]

AFAIK we've only ever sorted interfaces by name. The order in which the constants are currently defined is largely arbitrary anyway.

[candlerb]

There is an explicit sequence in the list constants e.g. RACK_STATUS_CHOICES, so it would be possible to map back to the index within this list, and sort on that if required.

[jeremystretch]

I should point out that there will almost certainly be a transition period where the API will display and accept both the numeric and string values.

Currently, a choice field is represented like this in the API:

"status": {
    "value": 1,
    "label": "Active"
}

And after the change to slugs, it will look like this:

"status": {
    "value": "active",
    "label": "Active"
}

While it would be possible (although perhaps difficult) to accept either integer and slug (string) values on write requests to maintain backward compatibility, obviously we cannot convey both values in the same field simultaneously.

One thought I had was to introduce a temporary setting, allowing an admin to toggle between the old integer values and the new slug values. This would let the user to designate their own "flag day" for the API change irrespective of the NetBox release cycle. However, I'm not confident that the additional work needed to support this will ultimately be worthwhile, since users are in control of when they upgrade anyway.

[tyler-8]

Honestly I'm conflicted on this one. My main concern would be unintended side-effects in terms of database performance at scale. Seeing that this would be a fairly significant breaking change, if decided as the direction to go, perhaps add some sort of warnings or recurring deprecation message in multiple areas and table it for 2.8? Based on past 2.X releases that would give the community ~3-6 months to adjust their code and workflows after the official decision.

[candlerb]

Database performance is not an issue. There is a massive python-based stack on top; the difference in time to read a string versus an int from the database will be immeasurable in comparison.

Breaking the API for reads bugs me. To avoid it, you'd have to have something like a query-string parameter which selects whether you want numeric or string values - it defaults to numeric initially, and later the default changes to string.

Is there really much value in this? An API is an API, hidden from humans. The "label" is what people see. Any sort of i18n implies that you must keep a mapping from human-readable labels to these codes.

[jeremystretch]

Is there really much value in this?

Yes. People have expressed understandable frustration around having to maintain local integer-to-label mappings for API fields, but what prompted me to open this issue is the need for human-friendly names for interface and port types in support of a device type library (#451).

[tyler-8]

Database performance is not an issue. There is a massive python-based stack on top; the difference in time to read a string versus an int from the database will be immeasurable in comparison.

My concern lies more with indexing (and searching) at scale based on strings vs integers on the database side, rather than Python reading a string or integer. FWIW I think you're probably right though. The cardinality of constants is relatively low (at least for now...)

I'm leaning on the side of keeping existing behavior though. It's mostly a non-issue as is with the choices API endpoints. This seems like a big API change for what amounts to saving maybe one API call. IMO, the amount of work required to change existing integrations with Netbox's API is greater than the amount of work to call the choices API now.

[cimnine]

First, I'm really in favor of a change in this direction. But I would like to raise my concern about the specific proposal of changing the value from being an id to a slug:

"status": {
-   "value": 1,
+   "value": "active",
    "label": "Active"
}

As a user of API clients and former author of one, I would not be happy to see the type of the 'value' field change (from int to string). This might have unforeseen consequences on any statically typed API clients. They have to be re-written (that's the small part) and all the code, that goes with them, as well. Most of them would have to introduce special behavior to support Netbox versions prior and after this change.

Hence I suggest to introduce a 'slug' field instead (which might or might not get removed in the future):

"status": {
    "value": 1,
    "slug": "active",
    "label": "Active"
}

This introduces the possibility for having at least a grace period, in which both - new and old - are available. Also, it does not change the type of a field to string that was – until now and by definition – clearly an int.

Further, it would open the possibility to just leave the value as it is for the foreseeable future, as it doesn't actually hurt to have both, does it?

[candlerb]

I would like to throw in one other option: keep the values in the database as integers, but make them foreign keys into a separate enumeration table for each status type. This means that:

• the values remain integers, and no migration is required
• the values become user-manageable (you can add your own Status values) - this is a long-requested feature
• validity is enforced by the database
• the API can return both integer and slug, as suggested by @cimnine (which incidentally I agree with)

I believe this won't have any measurable impact on performance. The FK relationship creates an index on the tiny enum/slug tables only; it doesn't affect the structure of the main tables which link to them. And the small number of enum/slug rows will be cached in RAM.

[jeremystretch]

This introduces the possibility for having at least a grace period, in which both - new and old - are available.

This would require NetBox to support both values in parallel. This is a nonstarter for me as that's simply not a reasonable burden to place on the maintainers: we have to pick one or the other.

This might have unforeseen consequences on any statically typed API clients. They have to be re-written (that's the small part) and all the code, that goes with them, as well.

I think this is a reasonable requirement. The most popular API client is pynetbox, and implementing this change should be pretty straightforward.

This introduces the possibility for having at least a grace period, in which both - new and old - are available.

This is simply delaying a breaking change. Long-term we want to keep the value field as slug is less intuitive. So, we either change value from an integer to a slug in one go, or we introduce slug first and then eventually rename it to value for the same end result. It's much easier to manage one change than two.

[jeremystretch]

I would like to throw in one other option: keep the values in the database as integers, but make them foreign keys into a separate enumeration table for each status type.

The plan is to introduce enumerated choices for all ChoiceFields once we move to Django 3.0.

I believe this won't have any measurable impact on performance.

Introducing ForeignKey relationships incurs additional JOINs. This was the primary reason for using static values originally.

the values become user-manageable (you can add your own Status values) - this is a long-requested feature

I won't get into this as I've already provided my stance, but I will point out that we're talking about all choice fields, including those which have no reasonable use case for user-defined values, such as power phases or cable length unit.

[DanSheps]

I definitely support this change, it would also allow us to take care of some "binary" fields (connected/planned).

[lampwins]

For me, this is bringing up that we need to have a larger discussion around API versioning in general.

By definition this is a breaking change and absent either backward-compatible API versioning or a "grace period" in which the current integer values are supported, this puts certain classes of users in a tough spot. From the user's perspective, it is not as simple as saying "update your client."

That might be true if the only client in question is local ad-hoc scripts, but imagine cases where multiple other systems have been integrated with netbox. When the user wants to upgrade netbox, all of those clients have to be updated in sync, otherwise, things break with this kind of change. So if we don't allow a sort of grace period here, we force users into atomic upgrades across their automation infrastructures.

[jeremystretch]

So if we don't allow a sort of grace period here, we force users into atomic upgrades across their automation infrastructures.

But no one is being forced to upgrade to v2.7 immediately. It's true that development stops on v2.6, but it should be reasonable to run a stable release of the v2.6 train for some time. It's a trade-off: make the necessary adaptations in order to get new features. While I believe every reasonable effort should be made to maintain backward compatibility for some period, everyone seems to have a different idea of what's reasonable.

Maybe instead of spending effort on ensuring some measure of backward compatibility between releases, we throw it out completely and instead begin supporting multiple trains. For example, keep releasing v2.6 versions with bug fixes only (no new features) in parallel with v2.7. When v2.8 comes out, kill v2.6 and change v2.7 to bug fixes only. IMO this is likely to be less of a burden for maintainers and more convenient for customers need more time to adapt.

[tyler-8]

The problem still lies in the transition of dependent-services. If the solution is to "wait until everything supports the new scheme" then that means the release of N-number of new versions external scripts/apps/services at the exact same time as the Netbox upgrade. That's untenable for most environments.

to @lampwins point, if there was some sort of API versioning in place, for example v2.7.0 provided these two endpoints:

/api/dcim/interfaces/
/api/v2/dcim/interfaces/

That would allow each individual external app/service/script to migrate in its own time, on separate release windows from the Netbox upgrade. This is a far better experience all around and makes the upgrade process much easier to go through.

From a maintainer perspective, really it's just a separate API module at that point, the code for the two shall remain separate - and you only move new code to the api_v2.py module when there's actually a v2 - keeping it clear where things are.

[DanSheps]

DRF does support API versioning: https://www.django-rest-framework.org/api-guide/versioning/

However, my problem with versioning is, at some point we have to stop maintaining the old version otherwise we are expending a large amount of resources maintaining something that we ultimately moved on from.

[tyler-8]

we have to stop maintaining the old version otherwise we are expending a large amount of resources maintaining something that we ultimately moved on from.

Absolutely - and you can even set those deadlines (for example, v1 removed by v2.8), add depreciation messages to releases notes and documentation. This is a common workflow and I think would be familiar to many Netbox users. Ansible, pip, and countless other apps & libraries follow this same pattern.

[jeremystretch]

The problem still lies in the transition of dependent-services. If the solution is to "wait until everything supports the new scheme" then that means the release of N-number of new versions external scripts/apps/services at the exact same time as the Netbox upgrade. That's untenable for most environments.

Why not simply extend these clients to support both versions simultaneously? That is what's being proposed for NetBox.

Ultimately this is a debate about how to distribute development burden, and I don't think it's fair to expect an all-volunteer team to commit to all the additional perpetual work required to maintain multiple API versions. We can barely keep up with the one as it is.

[cimnine]

To me, introducing API versions because of this seems to be overkill. That's why my suggestion was to extend the API rather than introducing a radical change.

For what it's worth, I suggest rewriting those constants to actual Python enums:

So this

RACK_STATUS_RESERVED = 'active'
RACK_STATUS_AVAILABLE = 'planned'
RACK_STATUS_PLANNED = 'reserved'
RACK_STATUS_ACTIVE = 'available'
RACK_STATUS_DEPRECATED = 'deprecated'
RACK_STATUS_CHOICES = [
    [RACK_STATUS_ACTIVE, 'Active'],
    [RACK_STATUS_PLANNED, 'Planned'],
    [RACK_STATUS_RESERVED, 'Reserved'],
    [RACK_STATUS_AVAILABLE, 'Available'],
    [RACK_STATUS_DEPRECATED, 'Deprecated'],
]

would become that

# Instead of the RACK_STATUS_* constants
class RackStatus(ChoiceEnum):
  ACTIVE = (1, 'Active')
  AVAILABLE = (2, 'Available')
  # ...

# with this code somewhere central
from enum import Enum
class ChoiceEnum(Enum):
  def __new__(cls, id, str):
    obj = object.__new__(cls)
    obj._value_ = id 
    obj.str = str
    return obj

  def __str__(self):
    return self.str

  @property
  def slug(self):
    return self.name.lower()

  @classmethod
  def as_choice(cls):
    return map(lambda choice: [choice.slug, str(choice)], list(cls))

  @classmethod
  def for_slug(cls, slug):
    return cls[slug.upper()]

These enums can then be used like this:

>>> list(RackStatus)
[<RackStatus.ACTIVE: 1>, <RackStatus.AVAILABLE: 2>]
>>> RackStatus(1)
<RackStatus.ACTIVE: 1>
>>> RackStatus.ACTIVE
<RackStatus.ACTIVE: 1>
>>> RackStatus['ACTIVE']
<RackStatus.ACTIVE: 1>
>>> RackStatus.for_slug('active')
<RackStatus.ACTIVE: 1>
>>> RackStatus['active'.upper()]
<RackStatus.ACTIVE: 1>
>>> list(RackStatus.as_choice())
[['active', 'Active'], ['available', 'Available']]

If other functionality is required, it can be added relatively easily to the ChoiceEnum. (E.g. if the slug can't always be inferred from the enum name like it's implemented above.)

A solution like the proposed would make it trivial to maintain the id, the corresponding slug and the actual name, for the foreseeable future:

class RackStatus(ChoiceEnum):
  ACTIVE = (1, 'Active')
  AVAILABLE = (2, 'Available')
  # ...

It would require a little more effort on the API side I believe. But I think it would boil down to a rather generic solution as well.

[jeremystretch]

For what it's worth, I suggest rewriting those constants to actual Python enums

This won't address the root issue of accepting and conveying multiple values simultaneously in the REST API. (Additionally, Django 3.0 will introduce purpose-built enumerated choices.)

[jeremystretch]

We need to decide what to do about this as it's currently blocking work needed for v2.7. We have a few options, which I'll list here.

Option 1: Change all integer choice values to slugs

Example:

"status": {
    "value": "active",
    "label": "Active"
}

This is the easiest option from a development standpoint, but of course it's also the most disruptive. Users would need to modify all affected API clients and upgrade NetBox to v2.7 at the same time.

Option 2: Introduce a new field to convey the slug value

Example:

"status": {
    "value": 1,
    "slug": "active",
    "label": "Active"
}

This approach would be the least disruptive, however it would either lock us into using slug as the value field instead of value, or it would impose yet another migration at some future point when slug gets renamed to value.

Option 3: Introduce a new field to convey the integer value

Example:

"status": {
    "value": "active",
    "integer": 1,
    "label": "Active"
}

This is similar to option 2, but it ensures a smooth future deprecation of the integer field by front-loading the breaking change. Affected clients would need to be updated to reference the new integer field if they are not able to immediately adopt slugs.

Option 4: Implement a configuration parameter to toggle between integer and slug values

This approach would introduce a mechanism by which a NetBox administrator can configure the API to use either integers or slugs in the value field. It is essentially a stop-gap measure to allow users to upgrade to a v2.7 release without needing to immediately update API clients to use slugs. However, it would still eventually require a "flag day" to switch from integers to slugs.

Option 5: Do nothing

We move forward with the v2.7 release without any changes to the existing integer values. This is acceptable, although it would be very nice to have made this switch prior to the implementation of #451, #792, and #1865.

[angely-dev]

Just a user perspective here. We use the API intensively and we also support this change. We have what @lampwins called ad-hoc scripts (more a tiny framework though) so option 1 is not a problem. NetBox is a tool with a lot of activity so it seems to me users should be ready to make some changes from one version to another, especially if it is blocking some value-added features. Otherwise, they can stick to the current version until ready.

Also, why not extend this on user-defined roles? I mean, it is possible to filter prefixes using a role slug but the ID has to be used it to create a new one using the API. Just asking.

[sdktr]

If option 1 is the intended outcome in the long run I'd prefer going that route in 2.7 directly. This is not very nice for users depending on external API client updates (and the synchronised 'big-bang-go-live') but imho the only feasible 'grey period' migration strategy boiles down to API versioning which @jeremystretch reasoned is unlikely to happen. We should act upon currently available realistic maintenance burden and I'd prefer not creating any future work in an already overbooked schema.

/offtopic API-versioning

Having a small (e.g. 1 minor version) overlap where people can utilize both the current and previous API might be less of an impact to the maintainers than keeping the complete current and previous minor releases up to date. But I'm in no position to make the correct judgement here so will stand corrected.

E.g.: Netbox v2.7 supports both API schemas 2.6 and 2.7. If your client is 100% compatible with 2.7, it will continue to run in 2.8. If no breaking changes are in 2.9 it can still work, but the client still sends it 'known compatible schema' X-Api-Version: 2.7 and the server determines whether that endpoint version is still available.

[jeremystretch]

Also, why not extend this on user-defined roles?

Out of scope for the current discussion. This has been discussed and laid to rest in other issues.

offtopic API-versioning

This is off the table as we simply don't have the development resources to support it at present.

[jeremystretch]

After giving this some more thought, I'm going to proceed with option 3 as it seems like the best compromise between developer and user maintainability. The value field will convey the new slug value, and we'll add an id field to continue conveying the numeric value (to be deprecated for v2.8).

"status": {
    "value": "active",
    "id": 1,
    "label": "Active"
}

Importantly, these fields will accept both the numeric and slug values on write for the entire v2.7 release, to maintain backward compatibility for the near future.

[lukasodhner]

Will this change also apply to cable status? I believe this would relate to issue #3145 I know that cable status is currently boolean but if it was possible to add multiple statuses it would greatly improve our workflows. For example: -Decomissioning (connected) -Tested (disconnected) -Failed (disconnected) -Reterminate Side A(disconnected) -Reterminate Side B (disconnected) -Installation Error Side A (disconnected) -Installation Error Side B (disconnected)

I understand this may be more complex then changing the other enum entries but I really hope it can be included in the transition to varchar

[jeremystretch]

@lukasodhner Any changes to the available selections of any field are out of scope for this issue.

[lukasodhner]

@lukasodhner Any changes to the available selections of any field are out of scope for this issue.

Understood. I have a slightly modified version of Netbox in a sandbox and I am just curious if you plan on changing the cable status into a slug as part of this issue