Forced resync on login / resume

[jkitching]

Calling login() or resume() with sync=True (default) results in a "resync": local changes are destroyed, and a full download is made.

Using resync=True vs. resync=False doesn't make any difference when the cached state argument is not provided, since a full download needs to be made either way.

But when providing a state argument with cached state of Keep notes, both login() and resume() simply throw that state away and re-download. This is because, as mentioned above, the default sync=True implies a "resync".

To recap:

# A: Resyncs on login, but sync vs. resync here doesn't make a difference
keep.login(username, password)

# B: Throws state away and resyncs
keep.login(username, password, state=state)

# C: Keeps local state, and syncs to latest remote state
keep.login(username, password, state=state, sync=False)
keep.sync()

The current interface for providing a state (case B above) is a somewhat clunky user experience. Many users may believe their cached state is being used, when in fact it is just being thrown out the window.

Even switching sync=False is insufficient, since it completely disables any kind of sync on login. So the only resort is case C, which follows up with a manual sync(resync=False) call.

Solution

Unless there is a specific reason for the above design, may I suggest that the sync argument in login() and resume() be changed to imply a sync(resync=False) call instead of sync(resync=True)? (I.e. change the self.sync(True) call in load().)

In this case:

# A: Resyncs on login, but sync vs. resync here doesn't make a difference
keep.login(username, password)

# B: Keeps local state, and syncs to latest remote state
keep.login(username, password, state=state)

[kiwiz]

Will take a closer look at this once 0.15.0 is released.

[kiwiz]

Thanks for the thorough writeup! The self.sync(True) call was introduced in e8c38c45. I don't recall what the rationale was at that time.

[kiwiz]

This should be a low risk bugfix, though it might require users to be more intentional about handling exception.ResyncRequiredException

[jkitching]

Thanks, @kiwiz =)