Skip to content

Closes #19499 - Add WirelessLink Bulk Import Support by Device and Interface Names #19679

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pheus
Copy link
Contributor

@pheus pheus commented Jun 6, 2025

Fixes: #19499 - Wireless Link Bulk Import-Device/interface

  • Enhances the WirelessLink bulk import form to support specifying site and device for both terminations (A and B), with dynamic queryset filtering based on selection. Also allows interfaces to be referenced by name (instead of ID) during import, improving usability for CSV workflows.
  • Modifies the CSV data structure in WirelessLink test cases to include device names alongside interface names for both terminations.

pheus added 2 commits June 6, 2025 22:52
Enhances the WirelessLink bulk import form to support specifying
site and device for both terminations (A and B), with dynamic
queryset filtering based on selection. Also allows interfaces to
be referenced by name (instead of ID) during import, improving
usability for CSV workflows.
Modifies the CSV data structure in WirelessLink test cases to include
device names alongside interfaces for both terminations. Enhances
readability and aligns tests with import functionality updates.
@pheus pheus marked this pull request as ready for review June 9, 2025 18:12
@jnovinger jnovinger requested review from a team and arthanson and removed request for a team June 10, 2025 14:58
Copy link
Collaborator

@arthanson arthanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created the device and interfaces and did the import correctly, that all worked fine. I removed the WirelessLink that was created and re-did the import but I changed device_a to a different device that didn't have the interface and imported and got the crash below.

May be an existing issue that was exposed by these changes but looks like some more validation might be needed.

  File "/Users/ahanson/dev/work/netbox/netbox/netbox/views/generic/bulk_views.py", line 505, in post
    new_objs = self.create_and_update_objects(form, request)
  File "/Users/ahanson/dev/work/netbox/netbox/netbox/views/generic/bulk_views.py", line 463, in create_and_update_objects
    if model_form.is_valid():
       ~~~~~~~~~~~~~~~~~~~^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.13/site-packages/django/forms/forms.py", line 206, in is_valid
    return self.is_bound and not self.errors
                                 ^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.13/site-packages/django/forms/forms.py", line 201, in errors
    self.full_clean()
    ~~~~~~~~~~~~~~~^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.13/site-packages/django/forms/forms.py", line 339, in full_clean
    self._post_clean()
    ~~~~~~~~~~~~~~~~^^
  File "/Users/ahanson/dev/work/netbox/netbox/netbox/forms/base.py", line 78, in _post_clean
    return super()._post_clean()
           ~~~~~~~~~~~~~~~~~~~^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.13/site-packages/django/forms/models.py", line 498, in _post_clean
    self.instance.full_clean(exclude=exclude, validate_unique=False)
    ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.13/site-packages/django/db/models/base.py", line 1654, in full_clean
    self.clean()
    ~~~~~~~~~~^^
  File "/Users/ahanson/dev/work/netbox/netbox/wireless/models.py", line 201, in clean
    if self.interface_a.type not in WIRELESS_IFACE_TYPES:
       ^^^^^^^^^^^^^^^^
  File "/Users/ahanson/dev/work/netbox/venv/lib/python3.13/site-packages/django/db/models/fields/related_descriptors.py", line 271, in __get__
    raise self.RelatedObjectDoesNotExist(
        "%s has no %s." % (self.field.model.__name__, self.field.name)
    )

@pheus
Copy link
Contributor Author

pheus commented Jun 11, 2025

Thanks for testing that out and for the detailed feedback!

You're absolutely right - that crash shouldn’t happen. The issue you encountered when device_a didn’t have the specified interface is definitely concerning.

I’ll take a closer look at the interface validation. Adding an explicit check to ensure the interface exists on the given device before attempting to create the WirelessLink would definitely improve robustness here.

Really appreciate you catching that!

Adds checks for the presence of `interface_a` and `interface_b` to avoid
attribute errors during WirelessLink validation. Ensures robust handling
of edge cases where the attributes may be missing.
@pheus pheus force-pushed the 19499-add-wireless-bulk-import-support-by-device-and-interface-names branch from 99b1cf8 to 5114424 Compare June 11, 2025 14:07
@pheus
Copy link
Contributor Author

pheus commented Jun 11, 2025

I tracked down the root cause of the crash.

The issue stems from the model’s clean() method, which assumes that interface_a and interface_b are always set. However, during bulk import, it’s possible that one or both interfaces are missing (e.g., due to a typo or referencing a non-existent interface on the specified device). This causes an exception to be raised before the form validation can properly handle the error.

To resolve this, I’ve updated the clean() method to first check whether interface_a and interface_b are present before performing further validation. With this change, the crash no longer occurs, and the import form now correctly flags missing or invalid interfaces with a validation error instead of an unhandled exception.

Let me know if you'd prefer a different approach, but this seems like a safe and targeted fix that improves robustness.

@pheus pheus requested a review from arthanson June 11, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wireless Link Bulk Import-Device/interface
2 participants