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

The Natural Slug Property #4346

Closed
5 of 12 tasks
itdependsnetworks opened this issue Aug 29, 2023 · 11 comments · Fixed by #4439
Closed
5 of 12 tasks

The Natural Slug Property #4346

itdependsnetworks opened this issue Aug 29, 2023 · 11 comments · Fixed by #4439
Assignees
Labels
type: feature Introduction of new or enhanced functionality to the application
Milestone

Comments

@itdependsnetworks
Copy link
Contributor

itdependsnetworks commented Aug 29, 2023

TODO

From @lampwins comment below:

  • The natural key field(s) are still the input to the composite key function
  • Change the output of the composite key function to the django slugify function, or some derivative of it (look at our slugify_dashes_to_underscores function)
  • CSV Export will move to PK for the time being
  • Provide a clear acknowledgement that the resulting composite key strings will not have a guarantee of uniqueness.
  • The composite key strings will no longer be useable for direct lookup via any mechanism
  • Access this property via .natural_slug and should be accessible via the REST and GraphQL APIs
  • Append the first 4 characters of the PK to the end of the generated slug (if 314b709c-fa1e-46af-8c49-dfedd1f4081c append _314b) to aid in minimizing collisions

As ...

P.D. - Plugin Developer

I want ...

The ability to have a standard way to that "slugify" is provided. The request is to have a property that is called slug, based on name, and uniqueness is enforced based on this as well.

So that ...

  • My migration is easier
  • There is only one official way to "slugify" apps

I know this is done when...

There is a slug property on any model that has the "name" attribute on it.

Optional - Feature groups this request pertains to.

  • Automation
  • Circuits
  • DCIM
  • IPAM
  • Misc (including Data Sources)
  • Organization
  • Plugins (and other Extensibility)
  • Security (Secrets, etc)
  • Image Management
  • UI/UX
  • Documentation
  • Other (not directly a platform feature)

Database Changes

Yes, add properties

External Dependencies

No response

@itdependsnetworks itdependsnetworks added type: feature Introduction of new or enhanced functionality to the application triage This issue is new and has not been reviewed. labels Aug 29, 2023
@HanlinMiao HanlinMiao added question Further information is requested and removed triage This issue is new and has not been reviewed. labels Aug 29, 2023
@itdependsnetworks
Copy link
Contributor Author

Just to provide some color and concrete use cases, all of which could be handled in other manners, but trying to provide a reasonable user experience:

  • I use the slug for my file and folder structure, which has issues with special characters
  • I use my slug in my json/yaml/csv or other output formats that have issues with special characters
  • I use my slug in my url/html or other output tools that have issues with special characters
  • I prefer my shortened name slugified vs natural key since it is user intuitive, friendly and generally (but not always) is unique and handles 80%+ of my use cases. Meaning I do not have to think about what my device role, device type, or manufacturer strategy is going be, as I will rarely if ever have a name space issue here, but will have special characters.

@lampwins
Copy link
Member

After a few conversations, we are going to change some of the behavior of "composite keys" to to meet these usability concerns.

We will retain all of the underlying natural key functionality, however, the "composite key" forward and reverse functionality, based on a single string value, will be changed to the long standing "slugify" charset. The major reason for this is that the well intention implementation of the current composite key strings has created something that is not actually usable and thus users and developers are looking for something slightly different, hence this issue.

For this issue, we will:

  • The natural key field(s) are still the input to the composite key function
  • Change the output of the composite key function to the django slugify function, or some derivative of it.
  • Provide a clear acknowledgement that the resulting composite key strings will not have a guarantee of uniqueness.
  • The composite key strings will no longer be useable for direct lookup via any mechanism

We may:

  • Rename "composite key" to something else, likely "slug" related
  • Look to store the value in the database and allow non-unique lookups, if this need gains traction later

In the future we will:

  • Make enhancements to allow discovering metadata about a model's natural key requirements easier
  • Allow for API retrieval of objects via the natural key parts with a more direct structure, such as /ipam/vlans/<namespace_name>/<vlan_name>/ as an example, where a model's natural key field(s) become explicit URL parts that are properly documented in the API schema.

@lampwins lampwins removed the question Further information is requested label Aug 30, 2023
@lampwins lampwins added this to the v2.0.0 milestone Aug 30, 2023
@bryanculver
Copy link
Member

@glennmatthews
Copy link
Contributor

The composite key strings will no longer be useable for direct lookup via any mechanism

This may need some clarification - currently composite keys are used in CSV export/import as the default way of referencing related models. Are we proposing that CSV export will 1) change to export PKs for related models 2) change to explicit natural-key fields for related models or 3) remain as-is with some sort of approach to resolving non-unique composite-key-slugs?

@mzbroch
Copy link
Contributor

mzbroch commented Aug 30, 2023

  • Because this might implement an implicit data migration, it might require a management command in 1.6
  • Check "if your .slug" data will be changed in 2.0 if in 1.6 was not generated using slugify()

@bryanculver
Copy link
Member

  • Because this might implement an implicit data migration, it might require a management command in 1.6

    • Check "if your .slug" data will be changed in 2.0 if in 1.6 was not generated using slugify()

We should not call this .slug to prevent surprised behavior.

@smk4664
Copy link
Contributor

smk4664 commented Aug 31, 2023

Could we keep composite_key the way it is, a URL safe representation of the natural key, and then add a new field for the slugified natural key?

@bryanculver
Copy link
Member

Could we keep composite_key the way it is, a URL safe representation of the natural key, and then add a new field for the slugified natural key?

What's your use case for composite_key?

/cc: @lampwins

@smk4664
Copy link
Contributor

smk4664 commented Sep 1, 2023

Could we keep composite_key the way it is, a URL safe representation of the natural key, and then add a new field for the slugified natural key?

What's your use case for composite_key?

/cc: @lampwins

In NautobotChatOps, we have the user filter down to Devices, Racks, Circuits, etc. Currently, when the user chooses a device, the command used the PK as that is the lookup in 1.x. It would be great to be able to use the natural key instead. But it could be challenging switching to the natural key due to the way that the command input works in ChatOps.

That said, looking at more examples of composite_key that has special characters would make the command input challenging anyways. I would love to use the new property proposed, but it absolutely needs to be able to be used as a lookup.

@smk4664
Copy link
Contributor

smk4664 commented Sep 1, 2023

This said, I am leaning towards changing my position and instead use the natural key(s), but with a function that assists in taking the natural key output and joining it in a single string. Then when I need to us it as a lookup, splitting it back out into the natural key(s).

@bryanculver
Copy link
Member

This said, I am leaning towards changing my position and instead use the natural key(s), but with a function that assists in taking the natural key output and joining it in a single string. Then when I need to us it as a lookup, splitting it back out into the natural key(s).

That won't be possible or at the very least error-prone as the slug can't be guaranteed to be unique and it's inherently lossy: AMS 01, AMS_01, ams_01 would likely be slugged to ams_01.

@bryanculver bryanculver changed the title The slug property The Natural Slug Property Sep 1, 2023
@jathanism jathanism self-assigned this Sep 12, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature Introduction of new or enhanced functionality to the application
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants