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

Request to revert patch in pull request #10743 #7686

New Issue
Closed
opened 2025-12-29 20:26:58 +01:00 by adam · 6 comments
adam commented 2025-12-29 20:26:58 +01:00
Owner
Copy Link

Originally created by @cs-1 on GitHub (Feb 27, 2023).

NetBox version

v3.4.5

Python version

3.8

Steps to Reproduce

  1. Create a virtual chassis with at least two members, none of which is designated as master.
  2. Use any UI element in NetBox that calls the filter_device_id() function to gather interface information for a specific device (e.g. attach a cable from a device's front port to another device's interface; select a particular device in this UI element to only display interfaces of that device).

Expected Behavior

Only interfaces of a selected, non-master device are displayed (filtered).

Observed Behavior

All interfaces of all virtual chassis members are displayed despite selecting a single, non-master device.

The patch in pull request https://github.com/netbox-community/netbox/pull/10743 modifies filter_device_id() in a way that treats all members of a virtual chassis the same as a master chassis even if they aren't. This breaks all NetBox UI pages that rely on filter_device_id() because filtering interfaces by a single device doesn't work anymore. This functionality worked as intended before the above patch was applied.

(Please note that this issue was first identified in https://github.com/netbox-community/netbox/issues/11478 but it seems that this patch (also according to its author @arthanson) can have side effects in other parts of NetBox where filter_device_id() is used (not only when adding cables in the UI), hence the new issue to "untangle" the matter.)

Originally created by @cs-1 on GitHub (Feb 27, 2023). ### NetBox version v3.4.5 ### Python version 3.8 ### Steps to Reproduce 1. Create a virtual chassis with at least two members, none of which is designated as master. 2. Use any UI element in NetBox that calls the [`filter_device_id()` function](https://github.com/netbox-community/netbox/blob/561f1eadfc2386bd266b941d019d830c37c53cf3/netbox/dcim/filtersets.py#L1446-L1455) to gather interface information for a specific device (e.g. attach a cable from a device's front port to another device's interface; select a particular device in this UI element to only display interfaces of that device). ### Expected Behavior Only interfaces of a selected, non-master device are displayed (filtered). ### Observed Behavior All interfaces of all virtual chassis members are displayed despite selecting a single, non-master device. The patch in pull request https://github.com/netbox-community/netbox/pull/10743 modifies `filter_device_id()` in a way that treats all members of a virtual chassis the same as a master chassis even if they aren't. This breaks all NetBox UI pages that rely on `filter_device_id()` because filtering interfaces by a single device doesn't work anymore. This functionality worked as intended before the above patch was applied. (Please note that this issue was first identified in https://github.com/netbox-community/netbox/issues/11478 but it seems that this patch (also according to its author @arthanson) can have side effects in other parts of NetBox where `filter_device_id()` is used (not only when adding cables in the UI), hence the new issue to "untangle" the matter.)
adam added the type: bug label 2025-12-29 20:26:58 +01:00
adam closed this issue 2025-12-29 20:26:58 +01:00
adam commented 2025-12-29 20:26:59 +01:00
Author
Owner
Copy Link

@lp-2 commented on GitHub (Feb 27, 2023):

I have been trying to find a solution to problem #10610. I have been looking for possible solutions and I believe that the solution can be found in this section of the code: https://github.com/netbox-community/netbox/blob/develop/netbox/dcim/forms/model_forms.py#L1353-L1361.

In my opinion, the filter should be extended to include virtual chassis and LAG criteria. I have tried to implement this change, but I am having trouble understanding the NetBox structure. It seems that the solution requires further adjustments.

Any hints would be highly appreciated.

@lp-2 commented on GitHub (Feb 27, 2023): I have been trying to find a solution to problem [#10610.](https://github.com/netbox-community/netbox/issues/10610) I have been looking for possible solutions and I believe that the solution can be found in this section of the code: https://github.com/netbox-community/netbox/blob/develop/netbox/dcim/forms/model_forms.py#L1353-L1361. In my opinion, the filter should be extended to include virtual chassis and LAG criteria. I have tried to implement this change, but I am having trouble understanding the NetBox structure. It seems that the solution requires further adjustments. Any hints would be highly appreciated.
adam commented 2025-12-29 20:26:59 +01:00
Author
Owner
Copy Link

@DanSheps commented on GitHub (Mar 6, 2023):

We aren't going to revert, the solution would be to fix the offending code

@DanSheps commented on GitHub (Mar 6, 2023): We aren't going to revert, the solution would be to fix the offending code
adam commented 2025-12-29 20:26:59 +01:00
Author
Owner
Copy Link

@cs-1 commented on GitHub (Mar 7, 2023):

But the patch in the mentioned pull request breaks UI functionality and changes UI behaviour. @arthanson himself states in the pull request's comment that he doesn't know about side effects. We have found such a side effect. The patch changes the behaviour in a significant and unintended way. That's the reason for the request. The patch itself is the offending code. The code for the UI worked as intended until the patch in https://github.com/netbox-community/netbox/pull/10743 was applied.

@cs-1 commented on GitHub (Mar 7, 2023): But the patch in the mentioned pull request breaks UI functionality and changes UI behaviour. @arthanson himself states in the pull request's comment that he doesn't know about side effects. We have found such a side effect. The patch changes the behaviour in a significant and unintended way. That's the reason for the request. The patch itself is the offending code. The code for the UI worked as intended until the patch in https://github.com/netbox-community/netbox/pull/10743 was applied.
adam commented 2025-12-29 20:26:59 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Mar 7, 2023):

The code for the UI worked as intended until the patch in https://github.com/netbox-community/netbox/pull/10743 was applied.

It was not possible to assign lag interfaces on virtual chassis, so I don't think it did. There are two issues, the one you are having and the one the other guy in #10610 was having. You are saying break his use-case because yours is more important, while the correct approach is to make sure both issues are fixed and remain fixed.

@kkthxbye-code commented on GitHub (Mar 7, 2023): > The code for the UI worked as intended until the patch in https://github.com/netbox-community/netbox/pull/10743 was applied. It was not possible to assign lag interfaces on virtual chassis, so I don't think it did. There are two issues, the one you are having and the one the other guy in #10610 was having. You are saying break his use-case because yours is more important, while the correct approach is to make sure both issues are fixed and remain fixed.
adam commented 2025-12-29 20:27:00 +01:00
Author
Owner
Copy Link

@cs-1 commented on GitHub (Mar 7, 2023):

I understand what you mean. But it really isn't the case here. The patch in https://github.com/netbox-community/netbox/issues/10610 does not change code that directly deals with LAGs. It changes general filterset code that's used in different places all over NetBox, not only in the LAG assignment code. The LAG assignment code is found here https://github.com/netbox-community/netbox/blob/develop/netbox/dcim/forms/model_forms.py#L1353-L1361 But that's not where the LAG assignment problem is fixed. Changing a general filterset that's not exclusively used for LAG inevitably will have side effects as presented above. This has nothing to do with "my problem is more important". There's evidence that changing the filterset behaviour breaks functionality that used to work as intended and doesn't after the patch.

@cs-1 commented on GitHub (Mar 7, 2023): I understand what you mean. But it really isn't the case here. The patch in https://github.com/netbox-community/netbox/issues/10610 does **not** change code that directly deals with LAGs. It changes general filterset code that's used in different places all over NetBox, not only in the LAG assignment code. The LAG assignment code is found here https://github.com/netbox-community/netbox/blob/develop/netbox/dcim/forms/model_forms.py#L1353-L1361 But that's not where the LAG assignment problem is fixed. Changing a general filterset that's not exclusively used for LAG inevitably will have side effects as presented above. This has nothing to do with "my problem is more important". There's evidence that changing the filterset behaviour breaks functionality that used to work as intended and doesn't after the patch.
adam commented 2025-12-29 20:27:00 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Mar 7, 2023):

The patch in https://github.com/netbox-community/netbox/issues/10610 does not change code that directly deals with LAGs. It changes general filterset code that's used in different places all over NetBox, not only in the LAG assignment code

And in the process of fixing the other issue (not sure why this one was needed) it will probably be solved a different way. The entire point is that it's unlikely that there will be a revert.

Canging a general filterset that's not exclusively used for LAG inevitably will have side effects as presented above.

I feel like you've said this 10+ times now. No one is confused regarding what the issue is, the issue (#11478) is accepted, and it will be fixed when someone volunteers a patch or one of the maintainers has time.

As the two issues have the same root cause, one of them will need to be closed, I think it makes sense that the old issue stays, so I'll close this one. Feel free to update the other issue with any new info that's present only in this issue (didn't see any, but might have missed it).

@kkthxbye-code commented on GitHub (Mar 7, 2023): > The patch in https://github.com/netbox-community/netbox/issues/10610 does not change code that directly deals with LAGs. It changes general filterset code that's used in different places all over NetBox, not only in the LAG assignment code And in the process of fixing the other issue (not sure why this one was needed) it will probably be solved a different way. The entire point is that it's unlikely that there will be a revert. > Canging a general filterset that's not exclusively used for LAG inevitably will have side effects as presented above. I feel like you've said this 10+ times now. No one is confused regarding what the issue is, the issue (#11478) is accepted, and it will be fixed when someone volunteers a patch or one of the maintainers has time. As the two issues have the same root cause, one of them will need to be closed, I think it makes sense that the old issue stays, so I'll close this one. Feel free to update the other issue with any new info that's present only in this issue (didn't see any, but might have missed it).
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 type: bug
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#7686
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?