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

exp apply: safeguard against unclean workspaces #6930

Closed
mattlbeck opened this issue Nov 4, 2021 · 7 comments · Fixed by #7910
Closed

exp apply: safeguard against unclean workspaces #6930

mattlbeck opened this issue Nov 4, 2021 · 7 comments · Fixed by #7910
Assignees
Labels
A: experiments Related to dvc exp bug Did we break something? p1-important Important, aka current backlog of things to do

Comments

@mattlbeck
Copy link
Contributor

mattlbeck commented Nov 4, 2021

dvc exp apply will clear the workspace of untracked files and unstaged modifications. To prevent users losing work, it would be good to follow git's convention of preventing overwriting of untracked/modified files unless the user gives explicit permission.

  • If dvc exp apply is executed on an unclean workspace, fail with a suggestion to stash work or specify a --force flag
  • dvc exp apply --force acts like the current command does.
@daavoo
Copy link
Contributor

daavoo commented Nov 4, 2021

Related: iterative/dvc.org#2397

@karajan1001 karajan1001 added A: experiments Related to dvc exp feature request Requesting a new feature labels Nov 4, 2021
@pmrowla
Copy link
Contributor

pmrowla commented Nov 15, 2021

This was the original behavior for exp apply, but after much discussion the conclusion was that a majority of typical DVC users would always be using exp apply --force. So the behavior was changed to force-apply by default (and the exp apply --no-force option was added for users who do not want the default behavior)

@mattlbeck
Copy link
Contributor Author

@pmrowla I hadn't noticed the --no-force argument. On reflection, switching between experiments would necessitate use of a --force flag if it existed, and I guess that use-case is considered the most common.

I will monitor how I use exp apply for now, and whether I can easily encorporate --no-force into my workflow.

@dberenbaum
Copy link
Contributor

@pmrowla Forcibly overwriting conflicts is intended as explained here and in iterative/dvc.org#2397, but it seems like experiments are overwriting everything in the workspace, even untracked files. This seems to conflict with iterative/dvc.org#2397 (comment).

Example:

$ dvc stage add -n mystage echo foo
Creating 'dvc.yaml'
Adding stage 'mystage' in 'dvc.yaml'

To track the changes with git, run:

    git add dvc.yaml

To enable auto staging, run:

        dvc config core.autostage true
$ dvc exp run
Running stage 'mystage':
> echo foo
foo
Generating lock file 'dvc.lock'
Updating lock file 'dvc.lock'

To track the changes with git, run:

    git add dvc.lock dvc.yaml

To enable auto staging, run:

        dvc config core.autostage true

Reproduced experiment(s): exp-396c5
Experiment results have been applied to your workspace.

To promote an experiment to a Git branch run:

        dvc exp branch <exp> <branch>

$ touch newfile
$ dvc exp apply exp-396c5
Changes for experiment 'exp-396c5' have been applied to your current workspace.
$ git status
On branch main
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        dvc.lock
        dvc.yaml

nothing added to commit but untracked files present (use "git add" to track)
$ ls
dvc.lock dvc.yaml

@pmrowla pmrowla added bug Did we break something? and removed feature request Requesting a new feature labels Dec 23, 2021
@pmrowla pmrowla added the p2-medium Medium priority, should be done, but less important label Jan 27, 2022
@dberenbaum
Copy link
Contributor

Another user asked about this in discord: https://discord.com/channels/485586884165107732/485596304961962003/963800799220211782.

@pmrowla What do you think about bumping priority of this? It seems dangerous and potentially surprising to overwrite completely unrelated untracked files.

@pmrowla
Copy link
Contributor

pmrowla commented Apr 14, 2022

Unrelated untracked files being removed sounds like a bug. As for the priority, I guess I'm not sure if that should be opened as a separate issue from this one? It's unclear to me whether this issue is about the removing untracked files bug or if it's a higher level discussion about whether --force or --no-force should be the default behavior

@dberenbaum
Copy link
Contributor

I guess I hijacked this ticket from @mattlbeck. As mentioned in #6930 (comment) and other comments above, we already have discussed the default behavior and it usually makes sense to leave the current defaults.

So let's either prioritize this issue to stop dropping unrelated untracked files, or close this one and open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: experiments Related to dvc exp bug Did we break something? p1-important Important, aka current backlog of things to do
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants