Implement lakectl local checkout #6360
Conversation
N-o-Z
added
area/lakectl
|
🎊 PR Preview ee2d74e has been successfully built and deployed to https://treeverse-lakeFS-preview-pr-6360.surge.sh 🕐 Build time: 0.016s
🤖 By surge-preview |
There was a problem hiding this comment.
Overall looks good,
Changes requested just because I would like to get an error of unknown ref before any changes were made
Furthermore, I'm having a hard time understanding what justification is there to first revert my changes and then get the commit. let's say I removed a big file that was also removed from the new branch I'm going to checkout why do I need to download it in order to delete it? can't checkout just go over my current directory and align it to the requested ref?
cmd/lakectl/cmd/local_checkout.go
Outdated
| if specifiedRef != "" { | ||
| newRemote := remote.WithRef(specifiedRef) | ||
| newHead := resolveCommitOrDie(cmd.Context(), client, newRemote.Repository, newRemote.Ref) | ||
| newBase := newRemote.WithRef(newHead) | ||
| // write new index | ||
| _, err = local.WriteIndex(idx.LocalPath(), newRemote, newHead) | ||
| if err != nil { | ||
| DieErr(err) | ||
| } |
There was a problem hiding this comment.
Please move these to top, in case the commit isn't found I would like to know it before I erase all my local changes
There was a problem hiding this comment.
Looks much better,
Just to make sure ( even though you mentioned) - how was this tested?
Still have my concerns according to corrupted data in case of error during the operation, we need to remember all the cases, and maybe we should mention them in the issue
I think we'll try a general approach for all lakectl local commands |
Closes #6348
Change Description
Background
Implement lakectl local command to sync local directory with the remote state, overwriting any local change
New Feature
Part of task #6239
Testing Details
Unit testing for tools, lakectl tests will be added for esti upon feature completion. SyncManager will be tested as part of the command (was tested manually)
Breaking Change?
No