Open jamesharr opened 11 months ago
I support the idea of returning a sync result rather then the object itself, I think this would make error handling more straight-forward. @glennmatthews/@Dav-C - any opinion here?
I haven't thought it through in depth, but conceptually it feels reasonable to me. :)
Is anything lost by not returning the object? I think the current implementation loosely follows the REST paradigm where the object is returned for immediate use and the lack of a returned object is an error that should be handled. In any case, I think the return from all the methods should be consistent to avoid confusion.
When calling update
or delete
, you are calling on the class instance itself, so you have the instance available in 100% of cases. The proposal for create
returns a tuple with the class and the SyncResult
, so that's covered as well
Environment
Proposed Functionality
In 1.9.0, the results of a sync are stored as a field in DiffSyncModel, and for most users this status is automatically set when calling the super() of the create()/update()/delete() methods.
There are a few things that are awkward about this API:
self
for success orNone
for an error (and to skip children).self
needs to be returned. IE: why is this just not a bool?At a conceptual level, it's also a bit weird to have the status stored on an object. In particular, how do you report the result of a delete()? If that object is no longer in the destination backend, then how can I read the result of that after a sync process? I think this is ultimately because the result is not a property of an object, it's the result of a process.
This Feature Request proposes:
Misc notes
Use Case
The most straight forward way for a user to read the result of a status is to embed that status object into the DiffElement.