Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend the use of natural ordering to additional models #11279

Closed
JohannesKreuzer opened this issue Dec 21, 2022 · 4 comments · Fixed by #18009
Closed

Extend the use of natural ordering to additional models #11279

JohannesKreuzer opened this issue Dec 21, 2022 · 4 comments · Fixed by #18009
Assignees
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement netbox status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Milestone

Comments

@JohannesKreuzer
Copy link

NetBox version

v3.4.1

Feature type

Data model extension

Proposed functionality

Extend Virtualisation Cluster model to use natualsorting by the cluster name.

Use case

Better sorting of the clusters without padding the numbers in the name.

Database changes

No response

External dependencies

No response

@JohannesKreuzer JohannesKreuzer added the type: feature Introduction of new functionality to the application label Dec 21, 2022
@jeremystretch
Copy link
Member

jeremystretch commented Dec 29, 2022

IMO it doesn't make sense to do this only for clusters as a discrete goal. For instance, #10644 proposes also implementing natural ordering for circuits.

We use natural ordering for a few models in NetBox currently; namely sites, racks, devices, device components, VMs, and VM interfaces. This is accomplished by including a "hidden" _name field which stores a naturalized representation of the object's name suitable for database ordering.

While I'm not opposed to enabling natural ordering on other models, if we decide to do so, we should:

  1. Re-evaluate the current implementation to determine if a better approach exists
  2. Identify all suitable models and implement the change on all
  3. Ensure that the implementation is scalable (e.g. via a mixin class vs. explicit field definitions as we do currently)

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Dec 29, 2022
@kkthxbye-code
Copy link
Contributor

  • Re-evaluate the current implementation to determine if a better approach exists

Maybe we can use postgres collation as there is support for natural ordering.

We would have to create the collation in a migration with raw sql I assume, like:

CREATE COLLATION natural_sort (provider = icu, locale = 'en@colNumeric=yes');

And then the collation can be specified on the field like:

name = models.CharField(
    max_length=100,
    db_collation='numeric'
)

It also seems possible to specify it at query time.

@jeremystretch jeremystretch changed the title Natual Sorting of Clusters Extend the use of natural ordering to additional models Jan 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Apr 6, 2023
@jeremystretch
Copy link
Member

Collation definitely seems like the path forward, if it proves feasible. I recall going down this road a bit in the past but ran into limitations on older PostgreSQL versions. Django 4.2 drops support for PostgreSQL 11 (see #12237), so maybe that will free us up some?

@jeremystretch jeremystretch added needs milestone Awaiting prioritization for inclusion with a future NetBox release and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation pending closure Requires immediate attention to avoid being closed for inactivity labels May 2, 2023
@jeremystretch jeremystretch added the status: backlog Awaiting selection for work label May 21, 2024
@arthanson arthanson added the complexity: medium Requires a substantial but not unusual amount of effort to implement label May 22, 2024
@jeremystretch jeremystretch added this to the v4.2 milestone Sep 26, 2024
@jeremystretch jeremystretch removed the needs milestone Awaiting prioritization for inclusion with a future NetBox release label Sep 26, 2024
@jeremystretch jeremystretch added v4.2 and removed v4.2 labels Oct 18, 2024
@jeremystretch jeremystretch added the netbox label Nov 1, 2024 — with Linear
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: backlog Awaiting selection for work labels Nov 14, 2024
jeremystretch added a commit that referenced this issue Nov 15, 2024
…8009)

* 11279 add collation

* 11279 add collation

* 11279 add collation

* 11279 add collation

* 11279 fix tables /tests

* 11279 fix tests

* 11279 refactor VirtualDisk

* Clean up migrations

* Misc cleanup

* Correct errant file inclusion

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement netbox status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants