search

networktocode / diffsync

A utility library for comparing and synchronizing different datasets.
https://diffsync.readthedocs.io/
Other
153 stars 27 forks source link

Don't allow shortname conflict in store #201

Open jakubkrysl opened 1 year ago

jakubkrysl commented 1 year ago

Environment

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
Kircheneer commented 1 year ago

Thanks for submitting! This would need to be handled in

As such, its probably best to handle this in the classes inheriting from BaseStore as then it would only need to be handled for add and update.