Skip to content

Conversation

@glennmatthews
Copy link
Collaborator

Fixes #6, I think.

In brief - you can now optionally specify a Diff subclass when calling diff_from, diff_to, sync_from, or sync_to. The main reason for doing this is that after creating and populating a Diff with all relevant data, we now call a complete() callback on the Diff object; by default this does nothing, but a subclass could, for example, respond to this callback by printing the completed diff, saving it to a file, or writing it to a database.

@glennmatthews glennmatthews changed the title Add complete() callback when creating Diffs, allow caller to specify custom Diff subclasses More features and refactoring Oct 7, 2020
@glennmatthews
Copy link
Collaborator Author

Additionally:

  • Add continue_on_failure option to sync_to/sync_from (Configurable abort-on-failure vs continue-on-failure #8)
  • Move the CRUD APIs from the DSync class to the DSyncModel class. This is a pretty substantial refactor but I think it puts us in a better position to move forward, as now we can have a small amount of CRUD logic per DSyncModel instead of a tremendous amount of CRUD logic in a single DSync class.
    • Just realized I haven't updated the README for this change - will do that soon.

@glennmatthews glennmatthews requested a review from dgarros October 8, 2020 17:43
Copy link
Contributor

@dgarros dgarros 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 overall, I'll try to integrate that with NI tomorrow to see how it's working with all these changes

@glennmatthews
Copy link
Collaborator Author

Sorry, one more. I started working on #9 and realized that it would require adding another boolean parameter to the sync and diff APIs, and decided to revise these APIs to take a single flags parameter rather than having a whole bunch of individual boolean args.

This latest update fixes #9 by adding two more flags, SKIP_UNMATCHED_SRC and SKIP_UNMATCHED_DST. I'm totally open to suggestions for renaming these - could also be something like NO_CREATE and NO_DELETE, or SKIP_MISSING_DST and SKIP_MISSING_SRC, or ...

I also added a dsync field on all DSyncModel classes, which can be set when calling DSyncModel.create() or will be set automatically when calling DSync.add(). This way we don't have to re-pass the DSync as a parameter when calling the DSyncModel update() and delete() APIs, and generally makes them a bit more intuitive, I think.

@dgarros dgarros self-requested a review October 9, 2020 09:38
Copy link
Contributor

@dgarros dgarros left a comment

Choose a reason for hiding this comment

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

The flags looks good, not sure about the names as well so let's go with that for now.
The other changes looks great too

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.

Support for Diff subclasses in diff generation

3 participants