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

Cable post_save signal cause exception in loaddata management command #2472

New Issue
Closed
opened 2025-12-29 18:19:08 +01:00 by adam · 6 comments
adam commented 2025-12-29 18:19:08 +01:00
Owner
Copy Link

Originally created by @Grokzen on GitHub (Mar 19, 2019).

Originally assigned to: @DanSheps on GitHub.

Environment

  • Python version: 3.6.x
  • NetBox version: 2.5.8

Background

Some background that will describe what we are doing and how we found this bug. At our production envrionment we do dumps of the data once a day for backup purposes. We use the djangos built-in dumpdata for this. It outputs a json file that we commit to git to see what data changes over time. We also use these snapshots as test data that we can load to test envrionment to build prod replicas and we subset this sometimes at our local dev machines to load and test features and data migrations before going into prod. We use the djangos built-in loaddata to read the json backups back from file into the database.

After we merged into our local fork and deployed a upgrade from 2.5.6 to 2.5.8 we started to see some error with loaddata where it would throw a exception when run. I dug around and thought it was a merge issue but i could not find any problems with the merge. So i dug into the data to sett if i could dig up the problem object and code. The problem object was identified and i tracked the problem to the root cause that was a signal that is emit on post_save on Cable object saves. This is the stack trace it output

https://gist.github.com/Grokzen/1bed27569b80cbe34b6522c7e9746db4

The important part is when the code enters into the signal

# dcim/signals.py L37
def update_connected_endpoints(...)

To reproduce this bug, i extracted and sanitized out all data from the json data dump into this minimal data dump that should trigger the bug. I got this bug to reproduce on a clean/unmodified 2.5.8 installation so i hope it can be replicated elsewhere.

Steps to Reproduce

  1. Take the content of this gist https://gist.github.com/Grokzen/8f3cbce805296471a8cb52188db7232d and save it to a db-dump.json file
  2. Start with a fresh and clean database. You need to
  • drop the database
  • create it again with a base migrate to get all tables and content types
  • Since the data dump includes all content types, you need to remove all entries in that table
./manage.py shell -i python << END
from django.contrib.contenttypes.models import ContentType
ContentType.objects.all().delete()
END
  1. Run python3 manage.py loaddata db-dump.json to start loading the data into the database

Expected Behavior

The data should be loaded by django into the database and usable w/o any exception thrown.

Observed Behavior

The exception as mentioned link above is thrown

When i PDB the exception i get the following from the signal frame

(Pdb) instance.termination_a
<Interface: TenGigabitEthernet1/2>
(Pdb) instance.termination_b
<FrontPort: Port 3>

(Pdb) instance.termination_a.model
*** AttributeError: 'Interface' object has no attribute 'model'
(Pdb) instance.termination_b.model
*** AttributeError: 'FrontPort' object has no attribute 'model'

Possible solution

Since the problem is in the signal that is implemented here, this is not a django specific problem with dumpdata/loaddata per say as they perform and resolve issues as expected.

Django does implement a feature that when you run management commands, django will inject a kwarg raw=True into all signals that is emited during the run of management commands.

The simplest solution would be to change it to the following

def update_connected_endpoints(instance, **kwargs):
    """
    When a Cable is saved, check for and update its two connected endpoints
    """
    if kwargs.get('raw', False):
        return False

But a more useful solution would be to make a decorator for it like

def disable_for_loaddata(signal_handler):
    @wraps(signal_handler)
    def wrapper(*args, **kwargs):
        if kwargs['raw']:
            return
        signal_handler(*args, **kwargs)
    return wrapper

@receiver(post_save, sender=Cable)
@disable_for_loaddata
def update_connected_endpoints(instance, **kwargs):
    """
    When a Cable is saved, check for and update its two connected endpoints
    """
    ....

then it can be reused if other signals show signs of the same problem and it is more explicit about it's purpose and what it does.

There might be a possible fix further down the stack, but i would argue that during loading of a datadump, the problem signal have no real need to run on each cable save as the data from the datadump is already in a good state.

Note

I can't say how we got the database to be in this state or the steps to really reproduce it from the GUI that would produce the same or a similar data dump file. All i could figure our from the person probably responsible was that he was trying to make patch panels work and he got the data to be in this state.

Originally created by @Grokzen on GitHub (Mar 19, 2019). Originally assigned to: @DanSheps on GitHub. ### Environment * Python version: 3.6.x * NetBox version: 2.5.8 ### Background Some background that will describe what we are doing and how we found this bug. At our production envrionment we do dumps of the data once a day for backup purposes. We use the djangos built-in `dumpdata` for this. It outputs a json file that we commit to git to see what data changes over time. We also use these snapshots as test data that we can load to test envrionment to build prod replicas and we subset this sometimes at our local dev machines to load and test features and data migrations before going into prod. We use the djangos built-in `loaddata` to read the json backups back from file into the database. After we merged into our local fork and deployed a upgrade from 2.5.6 to 2.5.8 we started to see some error with `loaddata` where it would throw a exception when run. I dug around and thought it was a merge issue but i could not find any problems with the merge. So i dug into the data to sett if i could dig up the problem object and code. The problem object was identified and i tracked the problem to the root cause that was a signal that is emit on `post_save` on Cable object saves. This is the stack trace it output https://gist.github.com/Grokzen/1bed27569b80cbe34b6522c7e9746db4 The important part is when the code enters into the signal ``` # dcim/signals.py L37 def update_connected_endpoints(...) ``` To reproduce this bug, i extracted and sanitized out all data from the json data dump into this minimal data dump that should trigger the bug. I got this bug to reproduce on a clean/unmodified 2.5.8 installation so i hope it can be replicated elsewhere. ### Steps to Reproduce 1. Take the content of this gist https://gist.github.com/Grokzen/8f3cbce805296471a8cb52188db7232d and save it to a db-dump.json file 2. Start with a fresh and clean database. You need to - drop the database - create it again with a base migrate to get all tables and content types - Since the data dump includes all content types, you need to remove all entries in that table ``` ./manage.py shell -i python << END from django.contrib.contenttypes.models import ContentType ContentType.objects.all().delete() END ``` 3. Run `python3 manage.py loaddata db-dump.json` to start loading the data into the database ### Expected Behavior The data should be loaded by django into the database and usable w/o any exception thrown. ### Observed Behavior The exception as mentioned link above is thrown When i PDB the exception i get the following from the signal frame ``` (Pdb) instance.termination_a <Interface: TenGigabitEthernet1/2> (Pdb) instance.termination_b <FrontPort: Port 3> (Pdb) instance.termination_a.model *** AttributeError: 'Interface' object has no attribute 'model' (Pdb) instance.termination_b.model *** AttributeError: 'FrontPort' object has no attribute 'model' ``` ### Possible solution Since the problem is in the signal that is implemented here, this is not a django specific problem with dumpdata/loaddata per say as they perform and resolve issues as expected. Django does implement a feature that when you run management commands, django will inject a kwarg `raw=True` into all signals that is emited during the run of management commands. The simplest solution would be to change it to the following ``` def update_connected_endpoints(instance, **kwargs): """ When a Cable is saved, check for and update its two connected endpoints """ if kwargs.get('raw', False): return False ``` But a more useful solution would be to make a decorator for it like ``` def disable_for_loaddata(signal_handler): @wraps(signal_handler) def wrapper(*args, **kwargs): if kwargs['raw']: return signal_handler(*args, **kwargs) return wrapper @receiver(post_save, sender=Cable) @disable_for_loaddata def update_connected_endpoints(instance, **kwargs): """ When a Cable is saved, check for and update its two connected endpoints """ .... ``` then it can be reused if other signals show signs of the same problem and it is more explicit about it's purpose and what it does. There might be a possible fix further down the stack, but i would argue that during loading of a datadump, the problem signal have no real need to run on each cable save as the data from the datadump is already in a good state. ### Note I can't say how we got the database to be in this state or the steps to really reproduce it from the GUI that would produce the same or a similar data dump file. All i could figure our from the person probably responsible was that he was trying to make patch panels work and he got the data to be in this state.
adam closed this issue 2025-12-29 18:19:08 +01:00
adam commented 2025-12-29 18:19:09 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Mar 19, 2019):

Thanks for the report, I will try and reproduce it.

However, it is worth noting we probably can't get rid of update_connected_endpoints as that updates connected interfaces on the interfaces table.

@DanSheps commented on GitHub (Mar 19, 2019): Thanks for the report, I will try and reproduce it. However, it is worth noting we probably can't get rid of update_connected_endpoints as that updates connected interfaces on the interfaces table.
adam commented 2025-12-29 18:19:10 +01:00
Author
Owner
Copy Link

@Grokzen commented on GitHub (Mar 19, 2019):

No that is very true, but you do not need the signal to be emited during a manage.py loaddata event as that load will import all objects and connections and setup everything exactly as it was in the source netbox installation. Adding that check will only block the signal from executing it's code during loaddata management command and nothing else.

@Grokzen commented on GitHub (Mar 19, 2019): No that is very true, but you do not need the signal to be emited during a `manage.py loaddata` event as that load will import all objects and connections and setup everything exactly as it was in the source netbox installation. Adding that check will only block the signal from executing it's code during loaddata management command and nothing else.
adam commented 2025-12-29 18:19:10 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Apr 8, 2019):

I finished reviewing the dump data, and I agree, we don't need to include the signals (at all even).

I am going to do this and stash it as a PR for @jeremystretch and @lampwins to review.

@DanSheps commented on GitHub (Apr 8, 2019): I finished reviewing the dump data, and I agree, we don't need to include the signals (at all even). I am going to do this and stash it as a PR for @jeremystretch and @lampwins to review.
adam commented 2025-12-29 18:19:10 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 3, 2020):

This looks related to #3849, where the order in which the objects are exported prevents them from being cleanly imported. Unfortunately Django's dumpdata management command is not smart enough to infer inter-model dependencies: It merely exports models in the order it finds them. This can likely be fixed by simply rearranging the models as they appear in dcim/device_components.py.

@jeremystretch commented on GitHub (Feb 3, 2020): This looks related to #3849, where the order in which the objects are exported prevents them from being cleanly imported. Unfortunately Django's `dumpdata` management command is not smart enough to infer inter-model dependencies: It merely exports models in the order it finds them. This can likely be fixed by simply rearranging the models as they appear in `dcim/device_components.py`.
adam commented 2025-12-29 18:19:10 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 10, 2020):

Just noticed this was reported on v2.5.8. This is likely resolved by #3849 in v2.6.11. @Grokzen please try exporting and re-import data using release v2.6.11 or later and see if your issue is resolved.

@jeremystretch commented on GitHub (Feb 10, 2020): Just noticed this was reported on v2.5.8. This is likely resolved by #3849 in v2.6.11. @Grokzen please try exporting and re-import data using release v2.6.11 or later and see if your issue is resolved.
adam commented 2025-12-29 18:19:10 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Feb 11, 2020):

Can confirm that this does appear to be resolved by #3849 going to close this one out.

@DanSheps commented on GitHub (Feb 11, 2020): Can confirm that this does appear to be resolved by #3849 going to close this one out.
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
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#2472
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?