diffsync icon indicating copy to clipboard operation
diffsync copied to clipboard

Update `sync_from` and `sync_to` to return the diff and the status of the sync

Open dgarros opened this issue 2 years ago • 1 comments

Environment

  • DiffSync version: 1.3.0

Proposed Functionality

Currently the functions sync_to/from are not returning anything, whether the sync was completed or not. It would be useful to return at least the status of the sync and eventually the diff that was generated by the function.

Use Case

Developer experience

dgarros avatar Oct 28 '21 17:10 dgarros

This can be done here.

Kircheneer avatar Sep 29 '22 08:09 Kircheneer

This can be done here.

Is that issue a task wich can be handled by a new contributor?

dakonr avatar Nov 15 '22 15:11 dakonr

Yes absolutely @dakonr

In fact I had the hacktoberfest label on it until a couple of days ago. Feel free to submit a draft PR if you're unsure about something and you will get some feedback. Thanks!

Kircheneer avatar Nov 15 '22 18:11 Kircheneer

ok @Kircheneer I will try my best, you can assign that issue to me (I can't do that by myself)

First of all I need to understand the requirement and the intended behaviour.

  • The easy way could be to retrun always true at the end of the function. The code looks like, that on some stop points an error will raise.
  • An other solution is to return the diff. That can be used to check the sync and will interpret to true BUT also as false if there is no diff.
  • The third solution can return a tuple with state of the sync as first value and the diff as second value.

I couldn't find any clear design principles about that. Wich one will rise the DX @dgarros ?

dakonr avatar Nov 16 '22 20:11 dakonr

I just went through the code and with the way the API is currently structured I am not sure how we can get useful information about a sync status. Like you said, for example here we raise an exception rather then defining some error message to be returned. Since this is a common Python pattern I would prefer that we stick with it rather then returning the result information from the methods. Therefore I vouch for just returning the diff with the successful return of the function signaling execution success. Any differing opinion @glennmatthews?

Kircheneer avatar Nov 18 '22 08:11 Kircheneer

I think that makes sense, yeah.

glennmatthews avatar Nov 18 '22 15:11 glennmatthews

I agree with you both, the diff will be the best solution and will return more implicit information to the calling procedure. So, I will start with an implementation of that solution.

dakonr avatar Nov 18 '22 15:11 dakonr