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

Change logging for custom field deletion clean-up task #1631

Closed
briddo opened this issue Apr 12, 2022 · 8 comments · Fixed by #5567
Closed

Change logging for custom field deletion clean-up task #1631

briddo opened this issue Apr 12, 2022 · 8 comments · Fixed by #5567
Assignees
Labels
ms type: bug Something isn't working as expected

Comments

@briddo
Copy link
Contributor

briddo commented Apr 12, 2022

In addition to the work already done for #1582 on this PR, we need to add change logging for each updated object in nautobot.extras.tasks.delete_custom_field_data code here

@briddo briddo added the type: bug Something isn't working as expected label Apr 12, 2022
@glennmatthews
Copy link
Contributor

I think more generally this is a gap for all of our custom field background updates (such as populating default values for a newly created custom field).

@glennmatthews
Copy link
Contributor

I think a "complete" fix here would involve all of the following background task functions in nautobot.extras.tasks:

  • update_custom_field_choice_data should create ObjectChange records for each model instance impacted when changing the definition of a custom field choice.
  • delete_custom_field_data should create ObjectChange records for each model instance impacted when deleting a custom field (or simply removing its association from a previously assigned content-type(s))
  • provision_field should create ObjectChange records for each model instance impacted when adding a new custom field with a default value (or expanding the scope of such an existing custom field to a new content-type(s)).

For all of the above, if no data is changed on a given model instance, an ObjectChange should not be generated for that specific instance.

@gsnider2195
Copy link
Contributor

This is going to require some more effort than initially estimated.

  • Need to store the current user in something like thread local storage or contextvars so that m2m_changed signal and model save/delete methods can retrieve the user for change logging
  • Need to be able to pass the user to celery tasks so that the worker knows which user to attach to the ObjectChange. Probably a good idea to add this to the celery request object and use this in a custom nautobot task class similar to the work in [WIP] Implementation of singleton pattern for Jobs #1081

@gsnider2195
Copy link
Contributor

Some of this work was completed but depends on the change logging improvements in #2236 in order to log changes by the user responsible for triggering the background task.

2231.patch.txt

@lampwins
Copy link
Member

Probably just need to convert these custom celery tasks to system jobs.

@lampwins
Copy link
Member

Need to account for users in the ORM case.

@lampwins lampwins added the ms label Jan 29, 2024
@gsnider2195
Copy link
Contributor

gsnider2195 commented Feb 20, 2024

It probably doesn't make sense to make this a system job yet since we still don't support running jobs without a user (the ORM case here). We should be able to introspect nautobot.extras.signals.change_context_state to get the current user and pass it to the background task for the change logging.

@lampwins
Copy link
Member

lampwins commented Apr 4, 2024

Always attempt to the user by whatever means are available (change context for example). As a last resort, allow system jobs to run without a user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ms type: bug Something isn't working as expected
Projects
No open projects
Status: Blocked
7 participants