Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension always asks to enable Force Upload #1016

Closed
inliquid opened this issue Aug 21, 2019 · 5 comments
Closed

Extension always asks to enable Force Upload #1016

inliquid opened this issue Aug 21, 2019 · 5 comments

Comments

@inliquid
Copy link

I enabled just simple auto upload, but when I edit settings.json extension always says that version in Gist is newer and asks to enable force upload.
изображение

BTW: VS Code language is english, so why the heck does it show up in Russian?

Enabling of Force Auto-upload works, but it's a non-sense, I have only 1 single VS Code instance, and just installed the extension. Gist is saved by extension.
lastUpload seems to be correct but it's saved in UTC while I'm on a different time zone (+3).

@shanalikhan shanalikhan added this to the v3.4.3 milestone Aug 22, 2019
@karl-lunarg
Copy link
Contributor

lastUpload seems to be correct but it's saved in UTC while I'm on a different time zone (+3).

I don't think that this is the issue because the timestamps are in UTC in both the gist and local files.

It looks like the real problem is caused by faulty logic for the "safe upload" check.

As it stands, if there are multiple machines doing uploads, then any machine that did not do the last upload cannot upload unforced, even if it downloaded first. This essentially requires the use of forceUpload, which is bad.

Consider:

  • Machine A uploads settings to make revision 5 in the gist.
  • Later, Machine B downloads, makes a change, and (force) uploads settings to make revision 6 in the gist.
  • Later, Machine A downloads revision 6 and makes a change and tries to upload.

The upload fails because Machine A's lastUpload has the timestamp of revision 5, and the gist is on revision 6. The current code considers the gist as "newer" if gistLastUpload >= localLastUpload.

Machine A's lastDownload is for revision 6 and so it seems that the local lastDownload should be compared to the gist's lastUpload to determine if the local machine has the latest gist content.

Indeed, this is what is seen in the "safe upload" issue proposal, but not in the current code.

So, I think you want to abort the upload if forceUpload is false and either of these is true:

(I'm not sure if forceUpload overrides both of the above or not - but just keep it the way it is now.)

@Bodix
Copy link

Bodix commented Sep 3, 2019

Downgrading extension version to 3.4.1 is helped for me.

@borekb
Copy link

borekb commented Sep 5, 2019

I thought I'm hitting a different issue in #1035 but it might be this one.

For search, this is what the dialog says:

Sync: Gist has a newer or identical version of your settings. Do you want to enable forceUpload to override this?

At this point, I think it would be the easiest to revert #923 and implement it again, with the feedback from here and #1035.

shanalikhan pushed a commit that referenced this issue Sep 5, 2019
* Make IsGistNewer compare against latestDownload

Instead of comparing the gist's latestUpload time against the local
latestUpload time, this function should compare the gist time against
the local latestDownload time in order to determine if the cloud
settings are newer than the local settings.

Also, on an upload, set the local latestDownload time to the same
time as the upload since the settings are effectively "downloaded" on
an upload.

This protects against the following scenario:

- Machine A uploads settings to cloud
- Machine B downloads these settings
- Machine B changes settings and uploads them to the cloud
- Machine A downloads the settings
- Machine A changes settings and tries to upload them

The last upload attempt fails because A's last upload time is before
the cloud's last upload time, even though A downloaded the settings.
A forced upload is needed in order to upload the settings.
It therefore makes sense to compare against the local download time.

* Fix changed files check to work with snippets

When checking for the existance of files in the gist, use the
settings file "gistName" to lookup the file in the gist instead of
"fileName" in order to find any files in a subdir.  The gist uses "|"
as a separator in the "gistName" for files in a subdir and so using
a plain filename does not match.

This fixes the case where a user has snippets and does an upload with
no changes to settings or snippets.  The snippets are already up in the
gist.  The code that checks for files that are local and not in the gist
fails to match the fileName to the gistName and incorrectly concludes
that the file is not in the gist.  This causes the upload code to upload
all the files even if there were no changes, resulting in an "empty"
gist revision with only the upload timestamp changed.

* Fix uploadCancelled status bar message timeouts
@shanalikhan
Copy link
Owner

v3.4.3 Released. Let me know if there is any problem :)

@Fiona-zhu
Copy link

@shanaccelirate Today my another devices installed v3.4.3, after configing gist ID & token,even turning on "forced download". But it showed the same tips: "sync:reading settings online on the botton",while vscode did nothing actually. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants