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

Simplify/reduce natural keys for Job model #2611

Closed
2 of 12 tasks
Tracked by #1574 ...
glennmatthews opened this issue Oct 13, 2022 · 8 comments · Fixed by #3431
Closed
2 of 12 tasks
Tracked by #1574 ...

Simplify/reduce natural keys for Job model #2611

glennmatthews opened this issue Oct 13, 2022 · 8 comments · Fixed by #3431
Assignees
Labels
impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release type: feature Introduction of new or enhanced functionality to the application
Milestone

Comments

@glennmatthews
Copy link
Contributor

As ...

Cora - Coder

I want ...

To have a simpler natural key for the Job model. Currently the uniqueness criteria for Job, and hence its natural keys, are either of:

        unique_together = [
            ("source", "git_repository", "module_name", "job_class_name"),
            ("grouping", "name"),
        ]

which, among other things, result in the need for the slug field to support excessively long (320 characters at present - 20 source + 100 git_repository.name + 100 module_name + 100 job_class_name) autogenerated values.

As a part of #765 we should rethink this design.

So that ...

  • the slug field (if we even keep it, see Remove the usage of slug fields #2312) can be shortened to a more reasonable length
  • Jobs can be uniquely identified without needing to specify so many distinct fields to do so

I know this is done when...

  • Jobs can be uniquely identified by one or two reasonably short fields as a natural key (such as a globally unique name like many/most other models have - class_path as currently implemented fails the "reasonably short" requirement)
  • Nautobot gracefully handles (with appropriate user/admin-facing error messages, for example) the installation or discovery of new Jobs that would violate the natural key uniqueness (for example, adding another Job that, as implemented, requests that it use a name or other natural key that's already been claimed by another installed Job)

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, changes to the Job data model.

External Dependencies

None expected

@glennmatthews glennmatthews added the type: feature Introduction of new or enhanced functionality to the application label Oct 13, 2022
@glennmatthews glennmatthews mentioned this issue Oct 13, 2022
9 tasks
@bryanculver bryanculver changed the title Simplify/reduce natural keys for Job model Epic: Simplify/reduce natural keys for Job model Nov 2, 2022
@bryanculver bryanculver changed the title Epic: Simplify/reduce natural keys for Job model Simplify/reduce natural keys for Job model Nov 2, 2022
@bryanculver bryanculver added this to the v2.0.0 milestone Nov 3, 2022
@bryanculver bryanculver added the impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release label Nov 3, 2022
@lampwins
Copy link
Member

lampwins commented Mar 6, 2023

Let's keep this simple make the name unique and the data migration will append numbers if duplicate names are found.

@bryanculver
Copy link
Member

Startup check for potentially invalid data or pattern for appending to name

@gsnider2195 gsnider2195 self-assigned this Mar 14, 2023
@gsnider2195
Copy link
Contributor

Just to clarify, do we want to use the job class name or the job Meta.name?

Ex:

class ExampleJob(Job):

    # specify template_name to override the default job scheduling template
    template_name = "example_plugin/example_with_custom_template.html"

    class Meta:
        name = "Example job, does nothing"
        description = """
            Markdown Formatting

            *This is italicized*
        """

ExampleJob or Example job, does nothing?

@gsnider2195
Copy link
Contributor

gsnider2195 commented Mar 14, 2023

Right now the job model name field uses job_class.name which is:

    @classproperty
    def name(cls):  # pylint: disable=no-self-argument
        return getattr(cls.Meta, "name", cls.__name__)

If we keep this behavior, it allows you to explicitly set the name using Meta.name but it will fall back to cls.__name__. Would this be a problem when a job author wants to modify their Meta.name and a new job model is generated? Also, do we try to slugify the Meta.name for the slug field?

@glennmatthews
Copy link
Contributor Author

Given that name for most other models is explicitly a human-readable name, I personally think that restricting it to the job class name (i.e., CamelCase python identifiers only, no spaces or punctuation) would not be ideal from a UX perspective.

Would this be a problem when a job author wants to modify their Meta.name and a new job model is generated?

This kind of thing is why the current model also stores the job_class_name as a separate, non-editable attribute - the idea being that if you change the class name, it's detected as a new Job; if you change the meta name it's just an update to the same Job. Not saying that's necessarily the right/best approach, just noting the current behavior.

Also, do we try to slugify the Meta.name for the slug field?

My hope is that with the natural-keys work this question becomes moot :-) But in the short term, yeah, if name becomes a unique field, then slug should be able to derive directly from name like most other models do.

@gsnider2195
Copy link
Contributor

In that case it sounds like job_class_name should be used to uniquely identify the job model.

@gsnider2195
Copy link
Contributor

gsnider2195 commented Mar 15, 2023

Decisions made in parking lot:

  • continue to identify the job model on ("source", "git_repository", "module_name", "job_class_name") and keep this as a unique_together
  • remove the unique_together for ("grouping", "name")
  • make name unique
  • generate slug from name field (will no longer need custom slug function slugify_dots_to_dashes)

@gsnider2195 gsnider2195 linked a pull request Mar 17, 2023 that will close this issue
6 tasks
@bryanculver
Copy link
Member

Closed with #3431

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release type: feature Introduction of new or enhanced functionality to the application
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants