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.

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

Fixes: #11079 - Handle cables across multiple rear-port positions #13337

Merged
merged 24 commits into from
Sep 26, 2023

Conversation

DanSheps
Copy link
Member

@DanSheps DanSheps commented Aug 1, 2023

Fixes: #11079 - Add supported path through modules and multiple rear-ports with different positions on same device

  • Add support for pathing through modules.
  • Adds some logic to generate split paths when path lengths vary

@DanSheps
Copy link
Member Author

DanSheps commented Aug 1, 2023

Not sure how I feel about the logging, could definitely remove it or modify it if someone has strong opinions.

jsenecal

This comment was marked as outdated.

@jsenecal

This comment was marked as outdated.

@DanSheps
Copy link
Member Author

DanSheps commented Aug 1, 2023

Don't create the path. It would fail normally anyways, this just allows the cable to be created but not create the cable path.

@jeremystretch
Copy link
Member

This is not a sufficient means of addressing the underlying issue. The assertions function as sanity checks: If they are being violated, we need to determine why and resolve accordingly. This might entail removing the assertions, or implementing other logic to accommodate the unexpected case. But merely logging them and avoiding creating the cable path is not a solution.

@DanSheps
Copy link
Member Author

DanSheps commented Aug 4, 2023

I am moving to a bit of a rewrite to properly account for at least this type of cable path. It should be ready for re-review in a few days.

@DanSheps DanSheps marked this pull request as draft August 4, 2023 02:59
@jsenecal

This comment was marked as outdated.

@DanSheps
Copy link
Member Author

I think this is ready @jeremystretch

@DanSheps DanSheps changed the title Fixes: #11079 - Catch AssertionError's in signals. Handle accordingly Fixes: #11079 - Handle cables across different modules and with multiple rear-port positions Aug 31, 2023
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Show resolved Hide resolved
netbox/dcim/models/cables.py Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
@DanSheps
Copy link
Member Author

DanSheps commented Sep 8, 2023

I have some fine-tuning to do on a new SVG rendering and then I will push that code as well.

@DanSheps
Copy link
Member Author

Things to note:

  1. If there are 3 or more cables in the render, it will push the other stuff to the tooltip and only show the Cable ID/Name.
  2. I have made it so it only puts unique cables in the node list per node. It does this in order to preserve termination and cable ordering (otherwise you might have cables ordered incorrectly which results in weird lines

@jeremystretch jeremystretch changed the title Fixes: #11079 - Handle cables across different modules and with multiple rear-port positions Fixes: #11079 - Handle cables across multiple rear-port positions Sep 18, 2023
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
netbox/dcim/tests/test_cablepaths.py Outdated Show resolved Hide resolved
netbox/dcim/models/cables.py Outdated Show resolved Hide resolved
netbox/dcim/svg/cables.py Outdated Show resolved Hide resolved
netbox/dcim/svg/cables.py Show resolved Hide resolved
Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

This has been a huge undertaking. Thanks for seeing it through @DanSheps!

@jeremystretch jeremystretch merged commit f65744f into develop Sep 26, 2023
8 checks passed
@jeremystretch jeremystretch deleted the 11079-Cablepath_catch_assertions branch September 26, 2023 17:16
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants