networktocode/diffsync

Remove `diffsync` field from DiffSyncModel

Opened this issue · 4 comments

Environment

  • DiffSync version: 1.9.0

Proposed Functionality

  • Remove the diffsync field from DiffSyncModel
  • Add diffsync to DiffSyncModel.update and DiffSyncModel.create method signatures

There are a few reasons behind my thinking: 1) It makes the API a bit more consistent 2) It might make it easier for Python to run GC if there's no/fewer cyclical references between objects 3) I didn't see a lot of references to this field outside of create/update/delete

This is likely appropriate for diffsync v2 #232 since it breaks the API.

Other feature requests like #255 may benefit from this change since it would mean there is no assumption that DiffSyncModel has a reference to a given DIffSync

Use Case

class DiffSyncModel(BaseModel):
    @classmethod
    def create(cls, diffsync: "DiffSync", ids: Dict, attrs: Dict) -> Optional[Self]:
        # Method signature remains the same
        ...

    def update(self, diffsync: "DiffSync", attrs: Dict):
        ...

    def delete(self, diffsync: "DiffSync"):
        ...

This actually similarly relates to nautobot/nautobot-app-ssot#272 because of the job=self reference which is being passed in many of Nautobot's usage simply to pass a logger down to the model methods of create, update, delete.

What do you ultimately feel the need to pass the DiffSync down to each of the models? Since the DiffSync holds all the models which are having an operation performed on them?

Yeah primarily for me its about having the logging context available.

Yeah primarily for me its about having the logging context available.

I don't necessarily need the DiffSync or Logger in the model itself. I need the logger in the CRUD methods and it's also very helpful to have the DiffSync instance in the CRUD methods because that's where we wind up stashing some things like the Nautobot API client, NSO API client, DB connections

But should you? Or should you have a generic AdapterConfig object that let's each adapter define the parameters and what gets sent to them. A very rough crappy example... cardoe@ffca6ef