Skip to content

Conversation

arthanson
Copy link
Collaborator

@arthanson arthanson commented May 8, 2025

Fixes: #674

This is PR #675 with merge fixes from master.

@arthanson arthanson requested a review from jnovinger May 13, 2025 15:45
@arthanson arthanson marked this pull request as ready for review May 13, 2025 16:33
@jnovinger jnovinger changed the title 675 trace Fixes #674: Include more types in trace() method May 14, 2025
interface_connection = InterfaceConnection


class PowerFeeds(TraceableRecord):
Copy link
Member

@jnovinger jnovinger May 14, 2025

Choose a reason for hiding this comment

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

This is probably my lack of pynetbox knowledge showing through, but why do we need a separate sub-class of TraceAbleRecord for PowerFeeds? Is there some reason that the addition to uri_to_obj_class_map is not sufficient?

If it is required, i Is there a reason it's body is just pass versus some other configuration declaration?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think there are any sub-objects needs to be set to the correct type, it is like Cables or InterfaceConnection. Those have to set __str but PowerFeed has name field which is the default str.

Copy link
Member

@jnovinger jnovinger 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, with one exception--there's no way to trace a circuit termination. Circuit terminations are definitely included as part of a represented trace with this change, but you can not call .trace() on a CircuitTermination instance.

But, since the original FR was for them being represented, that seems out of scope for this.

@arthanson arthanson merged commit ff0fd63 into master May 14, 2025
9 checks passed
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.

Update uri_to_obj_class_map to support additional supported termination endpoints in trace() method
3 participants