Skip to content

Fixes #21960: Allow cable trace to continue through multi-position FrontPorts without position context#22083

Open
attoba wants to merge 1 commit into
netbox-community:mainfrom
attoba:fix/multi-position-trace
Open

Fixes #21960: Allow cable trace to continue through multi-position FrontPorts without position context#22083
attoba wants to merge 1 commit into
netbox-community:mainfrom
attoba:fix/multi-position-trace

Conversation

@attoba

@attoba attoba commented May 3, 2026

Copy link
Copy Markdown
Contributor

Fixes: #21960

When tracing encounters a multi-position FrontPort with an empty position_stack, the current logic treats it as a split condition and aborts the trace early.

For FrontPorts, valid PortMapping entries may still exist and can be followed without requiring positional disambiguation at this stage.

This change removes the early-abort condition for FrontPorts by allowing traversal to continue using non-positional PortMapping resolution. Split detection is deferred to later stages of the trace, where it is already evaluated based on the actual topology.

The existing behavior for RearPorts is preserved, as multi-position RearPorts can fan out to multiple FrontPorts connected to different paths. Without positional context, continuing traversal in this case could produce incorrect results.

@pheus pheus requested review from a team and jeremystretch and removed request for a team May 4, 2026 11:16
@jeremystretch

Copy link
Copy Markdown
Member

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jeremystretch's task in 2m 36s —— View job


Review

Potential concern: No test coverage for the fixed scenario

The removed code path (multi-position FrontPort + empty position_stackis_split = True) has no corresponding regression test added. The existing tests in test_cablepaths2.py cover multi-position FrontPort cases but always with position context supplied from a breakout cable (so position_stack is populated before the FrontPort is encountered). There is no test that exercises the trace originating from a point where FrontPort.positions > 1 and position_stack is empty, which is the exact scenario this PR fixes. Without a test, this behavior could regress silently.

Subtle behavioral concern: all positions pushed when stack is empty

After the fix, when FrontPort.positions > 1 and position_stack is empty, the code falls through to:

port_mappings = PortMapping.objects.filter(front_port__in=remote_terminations)

This returns all PortMapping records for the FrontPort — one per position. Then at line 953–954:

if any(t.positions > 1 for t in terminations):
    position_stack.append([mapping.rear_port_position for mapping in port_mappings])

All rear_port_position values from all mappings are pushed. In the test_202 topology (FP1 with 4 positions, 4 PortMappings), this pushes [1, 2, 3, 4] — meaning the trace will follow all four fiber positions on the downstream RearPort. The result is a split/complete path covering all endpoints rather than an incomplete/aborted trace. That's arguably more useful, but it's worth confirming this is the intended outcome for issue #21960 rather than just allowing a single-fiber trace to proceed.

The underlying asymmetry with RearPort (still aborts on empty stack) is well-reasoned per the PR description — a FrontPort always maps to exactly one RearPort, so there's no directional ambiguity; the concern is only whether the "fan out all positions" behavior on the downstream leg is acceptable.

@jeremystretch jeremystretch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@attoba could you please include tests to ensure that this change doesn't introduce a breaking change to existing behavior?

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.

@github-actions github-actions Bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending closure Requires immediate attention to avoid being closed for inactivity

Projects

None yet

2 participants