starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Interface.objects.order_naturally() enhancements #1256

New Issue
Closed
opened 2025-12-29 16:30:48 +01:00 by adam · 6 comments
adam commented 2025-12-29 16:30:48 +01:00
Owner
Copy Link

Originally created by @tarkatronic on GitHub (Sep 21, 2017).

Issue type

[X] Feature request
[X] Bug report
[ ] Documentation

Environment

  • Python version:
  • NetBox version:

Description

I have discovered a few issues/limitations in the order_naturally() method which I have had to address in a local project, and would be happy to submit a PR for if desired.

The first is a bug in the actual ordering. Given interfaces with the following names:

  • Ethernet1/3/1
  • Ethernet1/4
  • Ethernet1/5/1

I would expect order_naturally() to return them in that same order. However, it is instead returning them in the order:

  • Ethernet1/3/1
  • Ethernet1/5/1
  • Ethernet1/4

This is a side effect of the regular expressions used in the method being anchored to the end of the string.

Second, I have added support for a third slash in the name (Ethernet1/2/3/4).

And finally, because it is highly recommended you do not use the .extra() method, I have updated the method to use .annotate() and RawSQL() instead.

Again, I can have a pull request ready for this ASAP if desired, along with appropriate tests.

Originally created by @tarkatronic on GitHub (Sep 21, 2017). <!-- Before opening a new issue, please search through the existing issues to see if your topic has already been addressed. Note that you may need to remove the "is:open" filter from the search bar to include closed issues. Check the appropriate type for your issue below by placing an x between the brackets. If none of the below apply, please raise your issue for discussion on our mailing list: https://groups.google.com/forum/#!forum/netbox-discuss Please note that issues which do not fall under any of the below categories will be closed. ---> ### Issue type [X] Feature request <!-- Requesting the implementation of a new feature --> [X] Bug report <!-- Reporting unexpected or erroneous behavior --> [ ] Documentation <!-- Proposing a modification to the documentation --> <!-- Please describe the environment in which you are running NetBox. (Be sure to verify that you are running the latest stable release of NetBox before submitting a bug report.) --> ### Environment * Python version: <!-- Example: 3.5.4 --> * NetBox version: <!-- Example: 2.1.3 --> <!-- BUG REPORTS must include: * A list of the steps needed to reproduce the bug * A description of the expected behavior * Any relevant error messages (screenshots may also help) FEATURE REQUESTS must include: * A detailed description of the proposed functionality * A use case for the new feature * A rough description of any necessary changes to the database schema * Any relevant third-party libraries which would be needed --> ### Description I have discovered a few issues/limitations in the `order_naturally()` method which I have had to address in a local project, and would be happy to submit a PR for if desired. The first is a bug in the actual ordering. Given interfaces with the following names: * Ethernet1/3/1 * Ethernet1/4 * Ethernet1/5/1 I would expect `order_naturally()` to return them in that same order. However, it is instead returning them in the order: * Ethernet1/3/1 * Ethernet1/5/1 * Ethernet1/4 This is a side effect of the regular expressions used in the method being anchored to the end of the string. Second, I have added support for a third slash in the name (`Ethernet1/2/3/4`). And finally, because [it is highly recommended you do not use the `.extra()` method](https://docs.djangoproject.com/en/1.11/ref/models/querysets/#extra), I have updated the method to use `.annotate()` and `RawSQL()` instead. Again, I can have a pull request ready for this ASAP if desired, along with appropriate tests.
adam added the type: feature label 2025-12-29 16:30:48 +01:00
adam closed this issue 2025-12-29 16:30:48 +01:00
adam commented 2025-12-29 16:30:49 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Sep 22, 2017):

@tarkatronic great work, thanks for the contribution! Unfortunately it still needs some work. With this approach, lo0 will appear between xe-0/9/9 and xe-1/0/0. I'm fiddling with it now. I merged the PR anyway because we definitely want the conversion from extra() to annotate() and the subposition field.

@jeremystretch commented on GitHub (Sep 22, 2017): @tarkatronic great work, thanks for the contribution! Unfortunately it still needs some work. With this approach, `lo0` will appear between `xe-0/9/9` and `xe-1/0/0`. I'm fiddling with it now. I merged the PR anyway because we definitely want the conversion from `extra()` to `annotate()` and the subposition field.
adam commented 2025-12-29 16:30:49 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Sep 22, 2017):

Ok, I think I figured out the trick. The solution, as always, is to add more regex. Previously, we were treating the zero in e.g. lo0 as its "slot," which isn't quite appropriate for the reason in my previous comment. What we can do is catch the zero as a separate ID with the regex ^(?:[^0-9]+)([0-9]+)$, and tweak the slot regex to match on a trailing slash. Then, each interface will have either a slot or an ID, and we simply sort IDs after slots (because IMO it's more common to have lo0/fxp0-type interfaces show up after the physical interfaces).

I seem to have this working perfectly locally. I just need to clean it up a bit and extend your test.

@jeremystretch commented on GitHub (Sep 22, 2017): Ok, I think I figured out the trick. The solution, as always, is to add more regex. Previously, we were treating the zero in e.g. `lo0` as its "slot," which isn't quite appropriate for the reason in my previous comment. What we can do is catch the zero as a separate ID with the regex `^(?:[^0-9]+)([0-9]+)$`, and tweak the slot regex to match on a trailing slash. Then, each interface will have _either_ a slot or an ID, and we simply sort IDs after slots (because IMO it's more common to have lo0/fxp0-type interfaces show up after the physical interfaces). I seem to have this working perfectly locally. I just need to clean it up a bit and extend your test.
adam commented 2025-12-29 16:30:49 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Sep 25, 2017):

I'm happy with how this is working right now. This issue can be re-opened if any notices a particular problem with the new logic (please be sure to include instructions detailing the steps needed to reproduce).

@jeremystretch commented on GitHub (Sep 25, 2017): I'm happy with how this is working right now. This issue can be re-opened if any notices a particular problem with the new logic (please be sure to include instructions detailing the steps needed to reproduce).
adam commented 2025-12-29 16:30:49 +01:00
Author
Owner
Copy Link

@tarkatronic commented on GitHub (Sep 28, 2017):

@jeremystretch
So I'm doing some more looking and testing of this updated logic, and I'm not sure it's quite right.

Now, this data is purely theoretical; I'm not sure this could happen in a real scenario, but let's just suppose it could. You have the following:

Ethernet1/1
Ethernet1
Ethernet1/3
Ethernet1/4
Ethernet1/1/1
Ethernet1/2

Now, my expected/preferred ordering for this would be

Ethernet1
Ethernet1/1
Ethernet1/1/1
Ethernet1/2
Ethernet1/3
Ethernet1/4

However currently I am getting

Ethernet1/1/1
Ethernet1/1
Ethernet1/2
Ethernet1/3
Ethernet1/4
Ethernet1

This is due to a combination of two things. The first is the removal of the COALESCE calls, which turns the unmatched fields into NULL, which PostgreSQL will, by default, sort to the bottom of the stack.

The second part is due to the addition of the \/ in the SLOT_RE. For interfaces with no subslot, that match is failing.

So I've arrived at the following:

        type_re = r"SUBSTRING({} FROM '^([^0-9]+)')"
        id_re = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)([0-9]+)$') AS integer)"
        slot_re = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)([0-9]+)\/?') AS integer)"
        subslot_re = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/)([0-9]+)') AS integer), 0)"
        position_re = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/){{2}}([0-9]+)') AS integer), 0)"
        subposition_re = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/){{3}}([0-9]+)') AS integer), 0)"
        channel_re = r"COALESCE(CAST(SUBSTRING({} FROM ':([0-9]+)(\.[0-9]+)?$') AS integer), 0)"
        vc_re = r"COALESCE(CAST(SUBSTRING({} FROM '\.([0-9]+)$') AS integer), 0)"

Thoughts?

@tarkatronic commented on GitHub (Sep 28, 2017): @jeremystretch So I'm doing some more looking and testing of this updated logic, and I'm not sure it's quite right. Now, this data is purely theoretical; I'm not sure this could happen in a real scenario, but let's just suppose it could. You have the following: ``` Ethernet1/1 Ethernet1 Ethernet1/3 Ethernet1/4 Ethernet1/1/1 Ethernet1/2 ``` Now, my expected/preferred ordering for this would be ``` Ethernet1 Ethernet1/1 Ethernet1/1/1 Ethernet1/2 Ethernet1/3 Ethernet1/4 ``` However currently I am getting ``` Ethernet1/1/1 Ethernet1/1 Ethernet1/2 Ethernet1/3 Ethernet1/4 Ethernet1 ``` This is due to a combination of two things. The first is the removal of the `COALESCE` calls, which turns the unmatched fields into `NULL`, which PostgreSQL will, by default, sort to the bottom of the stack. The second part is due to the addition of the `\/` in the `SLOT_RE`. For interfaces with no subslot, that match is failing. So I've arrived at the following: ```python type_re = r"SUBSTRING({} FROM '^([^0-9]+)')" id_re = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)([0-9]+)$') AS integer)" slot_re = r"CAST(SUBSTRING({} FROM '^(?:[^0-9]+)([0-9]+)\/?') AS integer)" subslot_re = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/)([0-9]+)') AS integer), 0)" position_re = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/){{2}}([0-9]+)') AS integer), 0)" subposition_re = r"COALESCE(CAST(SUBSTRING({} FROM '^(?:[^0-9]+)(?:[0-9]+\/){{3}}([0-9]+)') AS integer), 0)" channel_re = r"COALESCE(CAST(SUBSTRING({} FROM ':([0-9]+)(\.[0-9]+)?$') AS integer), 0)" vc_re = r"COALESCE(CAST(SUBSTRING({} FROM '\.([0-9]+)$') AS integer), 0)" ``` Thoughts?
adam commented 2025-12-29 16:30:49 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Oct 10, 2017):

I've made some tweaks, and interfaces are now ordered as:

Eth1/1
Eth1/1/1
Eth1/1/1/1
Eth1/1/2
Eth1/2
Eth1

I want to keep interfaces without a slash in their name at the bottom of the list, because a device typically has either slotted or non-slotted physical interfaces. In the cases where both exist, the non-slotted interfaces tend to be "special" interfaces like em0. This also avoids problems like mixing physical and virtual interfaces in the list.

These changes will appear in v2.2. I'm going to close out this issue for now, but as is tradition it can be re-opened if further adjustments are needed.

@jeremystretch commented on GitHub (Oct 10, 2017): I've made some tweaks, and interfaces are now ordered as: ``` Eth1/1 Eth1/1/1 Eth1/1/1/1 Eth1/1/2 Eth1/2 Eth1 ``` I want to keep interfaces without a slash in their name at the bottom of the list, because a device typically has _either_ slotted or non-slotted physical interfaces. In the cases where both exist, the non-slotted interfaces tend to be "special" interfaces like `em0`. This also avoids problems like mixing physical and virtual interfaces in the list. These changes will appear in v2.2. I'm going to close out this issue for now, but as is tradition it can be re-opened if further adjustments are needed.
adam commented 2025-12-29 16:30:49 +01:00
Author
Owner
Copy Link

@tarkatronic commented on GitHub (Nov 20, 2017):

Hey @jeremystretch, interested in another small enhancement? In my local project, I've created a SubstrFrom db function (which I'm also attempting to get added to Django core). With that, I was able to eliminate RawSQL entirely! So I now have this method working with straight up ORM.

        fields = {
            '_type': SubstrFrom(field, self.TYPE_RE),
            '_id': Cast(SubstrFrom(field, self.ID_RE), IntegerField()),
            '_slot': Cast(SubstrFrom(field, self.SLOT_RE), IntegerField()),
            '_subslot': Coalesce(Cast(SubstrFrom(field, self.SUBSLOT_RE), IntegerField()), Value(0)),
            '_position': Coalesce(Cast(SubstrFrom(field, self.POSITION_RE), IntegerField()), Value(0)),
            '_subposition': Coalesce(Cast(SubstrFrom(field, self.SUBPOSITION_RE), IntegerField()), Value(0)),
            '_channel': Coalesce(Cast(SubstrFrom(field, self.CHANNEL_RE), IntegerField()), Value(0)),
            '_vc': Coalesce(Cast(SubstrFrom(field, self.VC_RE), IntegerField()), Value(0)),
        }
@tarkatronic commented on GitHub (Nov 20, 2017): Hey @jeremystretch, interested in another small enhancement? In my local project, I've created a `SubstrFrom` db function (which I'm also attempting to get added to Django core). With that, I was able to eliminate `RawSQL` entirely! So I now have this method working with straight up ORM. ```python fields = { '_type': SubstrFrom(field, self.TYPE_RE), '_id': Cast(SubstrFrom(field, self.ID_RE), IntegerField()), '_slot': Cast(SubstrFrom(field, self.SLOT_RE), IntegerField()), '_subslot': Coalesce(Cast(SubstrFrom(field, self.SUBSLOT_RE), IntegerField()), Value(0)), '_position': Coalesce(Cast(SubstrFrom(field, self.POSITION_RE), IntegerField()), Value(0)), '_subposition': Coalesce(Cast(SubstrFrom(field, self.SUBPOSITION_RE), IntegerField()), Value(0)), '_channel': Coalesce(Cast(SubstrFrom(field, self.CHANNEL_RE), IntegerField()), Value(0)), '_vc': Coalesce(Cast(SubstrFrom(field, self.VC_RE), IntegerField()), Value(0)), } ```
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label type: feature
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#1256
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?