recovering from auto save should check the commit-sha

[rokroskar]

The autosave branch should only be restored if the commit-sha of the autosave equals the requested commit-sha.

See errors reported in SwissDataScienceCenter/renku-ui#472.

[m-alisafaee]

In addition to commit-sha, branch must also match.

[m-alisafaee]

There is yet another bug here: If we have un-pushed commit, they are autosaved by using the commit-sha of the local un-pushed commit. Since, there is not corresponding commit-sha in the remote repo, there is not way to recover the autosaved branch because its commit-sha does not exist in the remote repo.
Before, this was not an issue because we weren't checking the commit-sha of the autosaved branch. But, with this new change this is an issue.

Possible solution would be to use the commit-sha of the remote repo when creating autosaved branches.

[m-alisafaee]

There is another potential bug: if the user creates a branch but doesn't push it, there is no way to recover from an autosave for that branch.

[rokroskar]

I thought it is using the commit-sha that the server was started from?

[m-alisafaee]

No, it uses the commit-sha of the current local branch. From renku-notebooks/helm-chart/renku-notebooks/templates/configmap.yaml::pre-stop.sh:

...
    CURRENT_BRANCH=`git rev-parse --abbrev-ref HEAD`
    CURRENT_SHA=`git rev-parse --short HEAD`
    AUTOSAVE_BRANCH="renku/autosave/$JUPYTERHUB_USER/${CURRENT_BRANCH}/${CURRENT_SHA}"
...

[rokroskar]

right, @jachro and I had some discussions about this - we need to think carefully what the expected behavior would be. My impression is that we should be saving/restoring based on the branch/commit that the server was started from.

[jachro]

@rokroskar why do you think it's better? I'd care that much about what the user did after starting the session. I think it could be quite confusing if a user is currently working on branch B but the autosave branch would be pointing to branch A (I know it's just a name).
In general, the start branch is for me just a starting point for a user but when the user goes during his/her work it's up to them. It could be that the start branch is effectively only for doing git checkout -b new-branch.

[m-alisafaee]

To me also it does not make sense to use the initial branch/commit. They might be stale at the time of autosave. The user has to start from that branch/commit to have the autosaved branch restored.

[m-alisafaee]

Actually, we need both local and remote commit-shas here. This is needed to restore an autosaved branch for un-pushed commits. The remote commit-sha is needed to check for autosave and local remote-sha is needed to restore the un-pushed commits.
At the moment, if we only use remote commit-sha, all un-pushed commits are reset and if we only use local remote-sha we cannot match the autosaved branch.

[m-alisafaee]

There is one side-effect here: assume that commit xyz is un-pushed and remote's HEAD points at abc. If the pod is deleted, the autosave branch will be .../autosave/user/master/abc/xyz. The user can only resume from abc branch. If s/he does it the current script will match the autosave and will resume it, BUT the HEAD points to xyz. So, notebook were asked to resume abc but it resumed xyz. This is what started this whole thing.
If this behavior is not acceptable, then we can resume abc but any commit after it will be reset (i.e. xyz is lost).

[rokroskar]

Right, unless we push the unpushed commits to remote, we can't resume from there. So maybe the autosave should be from whatever the latest commit that both local and remote branch have in common? In any case, it would be nice if the UI could show that an autosave exists for a particular branch/commit combo.

[lorenzo-cavazzi]

I agree, and it shouldn't be that difficult considering we use the branch name to identify the project, branch and commit.

Should we only inform the user about this or give him the possibility to choose between starting from the autosaved branch or throwing it away? I personally prefer the first option, since the user may want to check what is in the autosave first

[lorenzo-cavazzi]

P.S. we generally use the 8 chars commit, is there a reason why the autosave branch uses 7 chars instead?

[rokroskar]

Oh at some point we were using 7 characters in the notebook service...

[m-alisafaee]

Let's discuss this issue tomorrow and make a final decision about it.

[m-alisafaee]

We agreed that:

• To keep both local and remote commit-sha in autosaved branch name
• Check commit-sha of the spawned server against the remote commit-sha of the autosaved branch and recover only when they match (these two are the same except when there were some un-pushed commits).
• The resumed notebook will be on the requested commit (spawned commit-sha) except when there were some un-pushed local commits. In this case, it will be on the last local un-pushed commit.

[m-alisafaee]

Closed by #190