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.

Sign up for GitHub

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

Jump to bottom

Staged Changes: InventoryItem Fails Sync #13422

Open
minitriga opened this issue Aug 9, 2023 · 7 comments
Open

Staged Changes: InventoryItem Fails Sync #13422

minitriga opened this issue Aug 9, 2023 · 7 comments
Assignees
@minitriga
Labels
severity: medium Results in substantial degraded or broken functionality for specfic workflows status: blocked Another issue or external requirement is preventing implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@minitriga
Copy link
Contributor

NetBox version

v3.5.1

Python version

3.10

Steps to Reproduce

Currently when working with staged changes there is a edge-case where the staged changes failed on the merge.

I believe this is because of the serialiser removing key parts of the data required to recreate the device when merge() is called.

netbox/netbox/netbox/staging.py

Line 119 in f5a1f83

data = serialize_object(instance, resolve_tags=False)

And when serialize_object is called it removes all MPTT model fields:

netbox/netbox/utilities/utils.py

Line 157 in f5a1f83

# Exclude any MPTTModel fields

This causes the merge to fail because the MPTT fields are now null. In the staged changes table.

Error message:

>>> Branch.objects.all()[0].merge()
TEST
<RestrictedQuerySet [<StagedChange: Create dcim.manufacturer (182)>, <StagedChange: Create dcim.devicetype (177)>, <StagedChange: Create dcim.devicerole (120)>, <StagedChange: Update dcim.site (124)>, <StagedChange: Create dcim.device (9608)>, <StagedChange: Create dcim.device (9609)>, <StagedChange: Create dcim.inventoryitem (848)>, <StagedChange: Create dcim.inventoryitem (849)>]>
Create dcim.manufacturer (182)
Create dcim.devicetype (177)
Create dcim.devicerole (120)
Update dcim.site (124)
Create dcim.device (9608)
Create dcim.device (9609)
Create dcim.inventoryitem (848)
Traceback (most recent call last):
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.NotNullViolation: null value in column "lft" of relation "dcim_inventoryitem" violates not-null constraint
DETAIL:  Failing row contains (2023-08-09 12:54:07.062+00, 2023-08-09 12:54:07.062+00, {}, 848, Inventory Item 1, , , , , , null, f, null, null, null, null, 9608, null, null, null, null, null).


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/opt/netbox/netbox/extras/models/staging.py", line 52, in merge
    change.apply()
  File "/opt/netbox/netbox/extras/models/staging.py", line 107, in apply
    instance.save()
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/serializers/base.py", line 288, in save
    models.Model.save_base(self.object, using=using, raw=True, **kwargs)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/models/base.py", line 863, in save_base
    updated = self._save_table(
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/models/base.py", line 1006, in _save_table
    results = self._do_insert(
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/models/base.py", line 1047, in _do_insert
    return manager._insert(
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1791, in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 1660, in execute_sql
    cursor.execute(sql, params)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 102, in execute
    return super().execute(sql, params)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.IntegrityError: null value in column "lft" of relation "dcim_inventoryitem" violates not-null constraint
DETAIL:  Failing row contains (2023-08-09 12:54:07.062+00, 2023-08-09 12:54:07.062+00, {}, 848, Inventory Item 1, , , , , , null, f, null, null, null, null, 9608, null, null, null, null, null).

Script to recreate:

from netbox.staging import checkout
from extras.models import Branch
from dcim.models import Device, VirtualChassis, DeviceType, Manufacturer, Site, DeviceRole, InventoryItem

Branch.objects.all().delete()
branch = Branch.objects.create(name='Branch 1')

Device.objects.filter(name="test device").delete()
Device.objects.filter(name="test device1").delete()
VirtualChassis.objects.filter(name='VC1').delete()
DeviceType.objects.filter(slug='device-type-1').delete()
Manufacturer.objects.filter(slug='manufacturer-1').delete()
DeviceRole.objects.filter(slug='device-role-1').delete()


with checkout(branch):

    manufacturer = Manufacturer.objects.create(
        name='Manufacturer', slug='manufacturer-1')
    device_type = DeviceType.objects.create(
        manufacturer=manufacturer, model='Device Type 1', slug='device-type-1'
    )
    device_role = DeviceRole.objects.create(
        name='Device Role', slug='device-role-1'
    )
    site, _ = Site.objects.update_or_create(
        defaults={'name': "test"}, slug="test")
    device, _ = Device.objects.update_or_create(defaults={
        "name": "test device", "device_role": device_role, "device_type": device_type, "site": site}, serial="123")
    device1, _ = Device.objects.update_or_create(defaults={
        "name": "test device1", "device_role": device_role, "device_type": device_type, "site": site}, serial="543")
    inv1 = InventoryItem.objects.create(device=device, name='Inventory Item 1'),
    inv2 = InventoryItem.objects.create(device=device1, name='Inventory Item 2'),



branch.merge()

print("DONE")

Expected Behavior

If there is a serializer or argument to pass to the serializer that does not remove key fields to recreate objects from the staged changes table.

Observed Behavior

The serialized object has key fields removed which are required to save the object.
@minitriga minitriga added the type: bug A confirmed report of unexpected behavior in the application label Aug 9, 2023
@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: medium Results in substantial degraded or broken functionality for specfic workflows labels Aug 23, 2023
@minitriga
Copy link
Contributor Author

@jeremystretch Happy to do the work on this but what do you think the best approach to solve this is? Add a keyword argument mptt=True and then if mptt before the mptt stuff?

@jeremystretch
Copy link
Member

@minitriga it would probably suffice to force an MPTT recalculation after the merge. Unfortunately, with all the competing bug reports currently open, I'm not able to dedicate any time to this. Would you like to take ownership of this issue?

@github-actions GitHub Actions
Copy link

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 Dec 26, 2023
@github-actions GitHub Actions
Copy link

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 26, 2024
@minitriga minitriga mentioned this issue Apr 24, 2024
Closed
@minitriga
Copy link
Contributor Author

Reviving this as our customers need to sync models that are MPTT Models. Such as Location and Inventory Item. Recalculating didnt work so well as to rebuild the MPTT you have to have the items in the DB and the issue is adding them into the DB without the MPTT fields.

@jeffgdotorg
Copy link
Collaborator

Reopening and assigning since this remains valid and needed, and a PR is ready.

@jeffgdotorg jeffgdotorg reopened this May 14, 2024
@jeffgdotorg jeffgdotorg removed pending closure Requires immediate attention to avoid being closed for inactivity status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels May 14, 2024
@jeffgdotorg jeffgdotorg added the status: accepted This issue has been accepted for implementation label May 14, 2024
@jeremystretch jeremystretch mentioned this issue May 24, 2024
Open
@jeremystretch
Copy link
Member

jeremystretch commented May 24, 2024 •
edited

Marking this as blocked by #16290, which when implemented should fully resolve this bug. (Please also see my comment on the PR.)

@jeremystretch jeremystretch added status: blocked Another issue or external requirement is preventing implementation and removed status: accepted This issue has been accepted for implementation labels May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@minitriga minitriga

Labels
severity: medium Results in substantial degraded or broken functionality for specfic workflows status: blocked Another issue or external requirement is preventing implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

issue_13422: Ignore MPTT fields in serialisation if being used by staged changes minitriga/netbox
3 participants
@jeffgdotorg @jeremystretch @minitriga