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

Closes #436 (and #122?): Add options to disable smooth cables and physics #488

Merged

Conversation

dremerb
Copy link
Contributor

@dremerb dremerb commented May 2, 2024

Although both issues has status "under review", this maybe makes the decision to accept the issue easier.

PR adds settings to make cable renderings straight lines, as proposed in #436.
Line style is controlled by an option in the "Individual Options".
I am currently working with ~200 hosts and 500+ cables, this change makes the graph a lot clearer in my opinion.

Two side effects:

  • With physics enabled, there is a more movement in the graph - but I want to propose another PR that allows turning that off anyway, maybe that synergy works out fine
  • Since cables are straight lines now, they are "bundled" (Combine redundant connections #122). Or rather plotted on top of each other. The cable's shadows overlay each other as well, which may be considered "not pretty", but accidentally works well as an indicator of multiple connections between two ends.

Fixing the shadows could be done extracting edges.append(create_edge(...)) calls into a dedicated function and ignoring duplicate connections.

Copy link
Collaborator

@mattieserver mattieserver left a comment

Choose a reason for hiding this comment

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

Can you maybe change the name of the vars you added?
Maybe make it 'straight_cables' or something like that.

@mattieserver mattieserver self-assigned this May 2, 2024
Copy link
Collaborator

@mattieserver mattieserver left a comment

Choose a reason for hiding this comment

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

Looks good, will merge after testing

Copy link
Collaborator

@dreng dreng left a comment

Choose a reason for hiding this comment

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

You have overlooked the saved filters.

api/views.py, line 112 and below
views.py, line 713 and below.

Copy link
Collaborator

@dreng dreng left a comment

Choose a reason for hiding this comment

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

You have not taken get_query_settings() into account in utils.py.

netbox_topology_views/api/views.py Outdated Show resolved Hide resolved
netbox_topology_views/views.py Outdated Show resolved Hide resolved
netbox_topology_views/views.py Outdated Show resolved Hide resolved
netbox_topology_views/views.py Outdated Show resolved Hide resolved
netbox_topology_views/views.py Outdated Show resolved Hide resolved
netbox_topology_views/views.py Outdated Show resolved Hide resolved
netbox_topology_views/views.py Outdated Show resolved Hide resolved
netbox_topology_views/views.py Show resolved Hide resolved
netbox_topology_views/views.py Show resolved Hide resolved
netbox_topology_views/views.py Show resolved Hide resolved
@dreng
Copy link
Collaborator

dreng commented May 13, 2024

We need to finish compatibility with NetBox 4.0 first. This PR may need to be rebased after that.

@mattieserver
Copy link
Collaborator

I will try to test/merge this this weekend.

@mattieserver mattieserver merged commit aba0598 into netbox-community:develop Jul 8, 2024
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.

3 participants