-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Clean up code for showing CircuitTermination on interfaces #4812
Comments
I propose the following clean-ups: Example 1There is no good way to consistently determine the Example 2In this scenario I think it makes the most sense to store CircuitTermination Z as the Example 3Here a choice has to be made about what to put in the
I think this would make the most sense to the user. |
I don't think there should be inconsistency between the way the 'via Circuit ID' is displayed as presented in these 3 examples. I understand the challenges with Example 1, however that behavior is going to cause confusion with users. In a way, the more complete your circuit connection model, the less information the GUI gives. That seems counter-intuitive. Personally, I find the information that an interface is connected to a circuit more valuable than the distant-end interface (that may or may not exist) on the device detail page. If I need the distant-end information, then I could click the cable trace button to get that. This would also cause conflict with #4619 [Edit] I realize you also mentioned in the above issue that you were working on it. Wasn't sure if this issue was a separate workstream altogether or not. |
This would be a major change to NetBox and goes beyond this cleanup issue. It would require a different implementation of tracing connections and determining the endpoint. |
I'm not suggesting anything different than the way NetBox has displayed interfaces connected to circuits since I was stating my preference of which information I found more helpful (from the historical behavior of NetBox) when looking at a device detail page vs the cable trace page. Adding... I understand this is a complex data model and also appreciate that you're working to make it better. I wanted to voice my concern about the change to displaying |
What has changed is that cable traces ended at the circuit termination, and the assumption was made that every circuit would lead to one interface. However that assumption is not always true, for example when using multiplexed connections or multiple circuits after each other. The code was fixed to deal with that bad assumption, but the consequence (for now) is that the circuit isn't shown anymore. I have ideas for improving this, but they require a major overhaul of the way we trace paths and store the useful data about them. It would definitely be nice, but I don't think it's feasible for now. There are too many other issues that need to be handled for the next releases. |
Please check a dependency with #4851 |
Related to #4900 |
Thanks for working on that and clearly documenting the different behaviors. I think we're hitting 2 issues related to "Example 1". Please let me know if that's not the right place, or if I should provide more details. Issue 1: Issue 2: |
If there was a connected endpoint to the circuit termination, the interface would have it as its connected endpoint itself
Hopefully temporary until upstream issues are fixed see also: netbox-community/netbox#4812 netbox-community/netbox#4925 Change-Id: I4e5d7d0fcbee3f71e500880c0cfbfc12efe4992a
If there was a connected endpoint to the circuit termination, the interface would have it as its connected endpoint itself
This has been addressed under #4900. |
This template code relies on
trace()
not traversing circuits:netbox/netbox/templates/dcim/inc/interface.html
Lines 94 to 123 in 43d6104
Since e143158 this is no longer the case, which makes the template code partially obsolete. This needs to be cleaned up.
Example 1
Consider the following topology:
Old implementation
Before e143158 the signal handler to update
connected_endpoint
s would calltrace(follow_circuits=False)
. This would cause theconnected_endpoint
for Interface X to be CircuitTermination A (and vice versa), and theconnected_endpoint
for Interface Y to be CircuitTermination Z (and vice versa).When the template is showing the connected endpoint it will try to call
get_peer_termination
on it. Because the connected endpoint is a CircuitTermination this may succeed (depending on whether both CircuitTerminations exist for the Circuit, in this example it exists) and if it does the CircuitTermination on the other end of the Circuit will be used. If that CircuitTermination has a connected endpoint then that connected endpoint will be shown as the connected endpoint of the interface, and the circuit information will be displayed as "via …".So in the example above, for Interface X, the old code would have CircuitTermination A stored as the connected endpoint in the database, but it would display Interface Y via Circuit C in the user interface.
Current implementation
Since e143158 the
follow_circuits
parameter totrace()
no longer exists, and the implementation always follows circuits. So now theconnected_endpoint
of Interface X will be Interface Y (and vice versa). The code that displays the connected endpoint of the peer of a circuit termination if the original connected endpoint is a circuit termination is therefore obsolete (and considering the length of that sentence probably rightfully so).Example 2
Consider the following topology:
Old implementation
The old implementation would stop at CircuitTermination A and store that as the connected endpoint. The user interface would then find the other CircuitTermination through
get_peer_termination
and display that.Current implementation
Because circuits are now always traversed
trace
will now return these steps in the path:Because the last CableTermination is empty the
connected_endpoint
will be empty and the user interface will show "Not connected"Example 3
Consider the following topology:
Both the old and the new implementation handle this in the same way. What makes this a special case is that this is the only remaining situation where the
connected_endpoint
of CircuitTermination A is updated. When CircuitTermination Z is later added and connected to an interface theconnected_endpoint
of Interface X will be updated to reflect that but theconnected_endpoint
of CircuitTermination A will still be Interface X.This behaviour is inconsistent because it only occurs when there is only one CircuitTermination connected to the Circuit. When both CircuitTerminations are created before connecting the cable the
connected_endpoint
of CableTermination A will always remain empty.The text was updated successfully, but these errors were encountered: