diffsync icon indicating copy to clipboard operation
diffsync copied to clipboard

Don't allow shortname conflict in store

Open jakubkrysl opened this issue 2 years ago • 1 comments

Environment

  • DiffSync version: 1.7.0
  • Python version: 3.10.8
  • nautobot-ssot version: 1.2.0

Context

I have an Nautobot SSoT Job. There is an issue where DiffSync store handles objects by uid but later for diff switches uid for shortname as name of the element: https://github.com/networktocode/diffsync/blob/b9b3b7a765499b79b37de0bbd36374ec5dc9767d/diffsync/init.py#L335-L346 But there is no way to check if given shortname is already used in DiffSync store while adding it, I can only check for uid. And when I run diff calculation, it fails on shortname conflict, but catching ObjectAlreadyExists at this point aborts whole diff calculation.

Expected Behavior

Fail on step 2, DiffSync should not allow to add object to store if given shortname is already stored.

Observed Behavior

Fail on diff calculation, which is too late to do anything about it.

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/nautobot_ssot/jobs/base.py", line 332, in run
    self.sync_data()
  File "/usr/local/lib/python3.10/site-packages/nautobot_ssot/jobs/base.py", line 155, in sync_data
    self.calculate_diff()
  File "/usr/local/lib/python3.10/site-packages/nautobot_ssot/jobs/base.py", line 82, in calculate_diff
    self.diff = self.source_adapter.diff_to(self.target_adapter, flags=self.diffsync_flags)
  File "/usr/local/lib/python3.10/site-packages/diffsync/__init__.py", line 644, in diff_to
    return target.diff_from(self, diff_class=diff_class, flags=flags, callback=callback)
  File "/usr/local/lib/python3.10/site-packages/diffsync/__init__.py", line 626, in diff_from
    return differ.calculate_diffs()
  File "/usr/local/lib/python3.10/site-packages/diffsync/helpers.py", line 93, in calculate_diffs
    self.diff.add(diff_element)
  File "/usr/local/lib/python3.10/site-packages/diffsync/diff.py", line 60, in add
    raise ObjectAlreadyExists(f"Already storing a {element.type} named {element.name}", element)
diffsync.exceptions.ObjectAlreadyExists: ('Already storing a virtualmachine named *SHORTNAME*', )

Steps to Reproduce

  1. Have a model with shortname set
  2. Add 2 objects for that model with different identifiers but same shortnames
  3. Run diff calculation

jakubkrysl avatar Jan 25 '23 17:01 jakubkrysl