Skip to content

Conversation

jeremystretch
Copy link
Member

@jeremystretch jeremystretch commented Sep 4, 2025

Closes: #20241

  • Call save() twice for new cables (before and after updating the terminations)
  • Consolidate the logic for getting/setting A/B terminations into the new _get_x_terminations() and _set_x_terminations() methods
  • Add get_terminations() to Cable to return a mapping of all current CableTerminations
  • Move the logic for updating CableTerminations to a new update_terminations() method on Cable
  • Override serialize_object() on Cable to add a_terminations and b_terminations to the change data
  • Add a complementary deserialize_object() method (for completeness)
  • Update some testing utilities to specifically check for creation records (ignoring subsequent update records)

@jeremystretch jeremystretch force-pushed the 20241-cable-change-records branch from 1220d87 to f5e69e5 Compare September 8, 2025 19:22
@jeremystretch jeremystretch marked this pull request as ready for review September 9, 2025 14:03
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.

Is there any concern that providing multiple avenues to modify cable terminations could create conflicts when both approaches operate on the same cable? For example:

  # Request 1: UI user editing cable via new approach
  cable = Cable.objects.get(pk=1)  # On branch X
  cable.a_terminations = [interface1]
  # ... during double-save process

  # Request 2: Plugin or custom script using old approach on same branch
  CableTermination.objects.create(
      cable_id=1,
      cable_end='A',
      termination=interface2
  )

  # Request 1 continues: update_terminations() reconciles based on stale cable state

This seems like it could happen with simultaneous UI/API operations, plugins, or custom scripts that manipulate CableTerminations directly during the save window.

I realize this is probably rare since most API usage would go through the Cable endpoints anyway, but wanted to check if there should be any safeguards or if this is considered acceptable risk given the benefits.

@jeremystretch
Copy link
Member Author

All modifications typically occur within a transaction bound to the request, so I don't think that's much of a concern. At least, I don't think it's any more of a concern than prior to the change.

Tangentially related, there's an effort to discourage the direct modification of cable terminations (see #20295), a mantra which should be extended to custom scripts as well.

@jnovinger
Copy link
Member

All modifications typically occur within a transaction bound to the request, so I don't think that's much of a concern. At least, I don't think it's any more of a concern than prior to the change.

Tangentially related, there's an effort to discourage the direct modification of cable terminations (see #20295), a mantra which should be extended to custom scripts as well.

Ah, right, I forgot about that! Thanks for the clarification.

@jnovinger jnovinger merged commit 873372f into main Sep 9, 2025
10 checks passed
@jnovinger jnovinger deleted the 20241-cable-change-records branch September 9, 2025 16:56
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.

Record A & B terminations on changelog record for cable
2 participants