You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request teaches the git lfs push & git lfs pre-push commands to allow missing objects by default, and enforce that they exist with the --strict-mode flag.
The commands should still return with a non-zero exit code, but at least the other objects have a chance to be uploaded.
I think printing a warning and exiting cleanly is OK in this scenario, where --strict-mode is not given, because the command did what it was supposed to do given the expectations set by the caller.
Finally, double check that upload retries reset the progress meter properly.
I verified that this is what happens, but I'm unsure if it's required that I q.meter.Pause() and q.meter.Start() in order to log to stderr. I think that it is, but perhaps not? (cc @technoweenie)
Lastly, I think that this was originally targeted for v2.0.2 (from #2063), but perhaps it should just ship in 2.1.0 since --strict-mode is a new flag and this is technically a breaking change?
I don't think we need --strict-mode. The push commands should always be strict. They should just follow through the entire upload, uploading everything they can, before exiting with an error. By adding --strict-mode, the unsafe behavior (ignoring missing objects) is default.
I don't think we need --strict-mode. The push commands should always be strict. They should just follow through the entire upload, uploading everything they can, before exiting with an error. By adding --strict-mode, the unsafe behavior (ignoring missing objects) is default.
Good point, what do you think about changing --strict-mode to --allow-missing and leaving it to false by default?
They should just follow through the entire upload, uploading everything they can, before exiting with an error.
I think keeping in --allow-missing is a safe idea, so long as we continue to let the transfer queue operate until it's processed all objects andWait() has been called, only then can we exit with a non-zero code if --allow-missing is not given.
Third take -- let's remove --strict-mode (and the idea of --allow-missing) all together, and still allow lfs push and lfs pre-push to upload all objects that it can and exit in a dirty state if any of the objects in the pushset were missing.
This will allow
All of the LFS objects that do exist to travel to the remote as early as possible
A user to see all of the missing objects in one push instead of getting one error at a time
A user to "fix" the missing object later, and have a smaller push size when they complete the fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request teaches the
git lfs push&git lfs pre-pushcommands to allow missing objects by default, and enforce that they exist with the--strict-modeflag.From #2063:
I think printing a warning and exiting cleanly is OK in this scenario, where
--strict-modeis not given, because the command did what it was supposed to do given the expectations set by the caller.I verified that this is what happens, but I'm unsure if it's required that I
q.meter.Pause()andq.meter.Start()in order to log to stderr. I think that it is, but perhaps not? (cc @technoweenie)Lastly, I think that this was originally targeted for v2.0.2 (from #2063), but perhaps it should just ship in 2.1.0 since
--strict-modeis a new flag and this is technically a breaking change?/cc @git-lfs/core #2063
Closes: #2063