checkout/add: change warning messages to regular/info if behavior is expected

[dmpetrov]

$ dvc -V
0.75.0

$ dvc checkout
WARNING: data 'newfile.txt' exists. Removing before checkout.
WARNING: data 'ttt.txt' exists. Removing before checkout.

EDITED 12/18/19: The message is actually wrong! The files were not removed - they were replaced by a new version. It has to be fixed as well.

$ echo hello >> newfile.txt
$ dvc add newfile.txt
WARNING: Output 'newfile.txt' of 'newfile.txt.dvc' changed because it is 'modified'
100% Add|███████████████████████████████████████████|1.00/1.00 [00:00<00:00,  2.45file/s]

To track the changes with git, run:

	git add newfile.txt.dvc

Based on my understanding warnings are needed when you need to alert a user. It is not needed for routine operations with expected results. In both of these cases, the priority needs to be reduced to regular/info.

PS: Are there any other warnings like these?

[shcheklein]

I think it's a duplicate of this #2329 ?

Also, yet another evidence (as many others) that loggers are not meant to be used as a mechanism to built UI/UX on top - #1930. In this specific case, for example, the priority is already set to INFO. It's just happened that in the "loggers world" INFO includes WARNING and above, not other way around.

[dmpetrov]

I think it's a duplicate of this #2329 ?

This is a discussion about the mechanism in general. It won't change the verbosity level of these particular messages.

loggers are not meant to be used as a mechanism to built UI/UX on top

Logging is just not the right term. We should use output and verbosity level instead (like in #2329).

Separation is definitely needed if we decide to output logs into a separate space while keeping printing messages to output (where verbosity level still be needed).

In this specific case, for example, the priority is already set to INFO. It's just happened that in the "loggers world" INFO includes WARNING and above, not other way around.

Sorry, I'm not following this. Could you please clarify? Where is it set to INFO and why it is a problem in this specific case? My understanding - by default we should print all info messages and above.

[shcheklein]

It won't change the verbosity level of these particular messages.

probably I was confused by PS: Are there any other warnings like these? and the generic title.

change it to be "add: remove output modified warning message"?

My understanding - by default we should print all info messages and above.

hmm, may be I didn't understand the intention for the ticket ... Technical details aside, what is the outcome you'd like to get - remove the message completely, remove the WARNING prefix?

[dmpetrov]

remove the WARNING prefix?

Yes

[shcheklein]

Gotcha! Yep, disregard my comments then! (I would probably still change title, this one feels generic, at least specific add/checkout: remove unnecessary warning prefixes - would make it way more explicit :) )

@dmpetrov , @shcheklein , what about removing those messages at all?

WARNING: data 'newfile.txt' exists. Removing before checkout.
WARNING: data 'ttt.txt' exists. Removing before checkout.

I think it was a warning because DVC is "removing" data.

echo "foo" > foo
dvc add foo
echo "something else" > foo
dvc checkout
# user will lost "something else"

In my opinion, having those messages just clutters the UI. This is related to #2498 .

by the way, I'm +1 for preventing the user to checkout when it has a "dirty" workspace

[dmpetrov]

@MrOutis it is a good point we can definitely consider this.

To my mind, we should NOT make checkout silent. It should output ALL the changes (and what exactly happened: remove/changed/added) or at least a summary. I'd prefer to see all the changes.

Re checkout... I've just noticed that the message is wrong (I'll modify the description) - in this scenario, the files were not removed - they were replaced by a new version. So, the message needs to be changed. Also, we need to make sure that the message will be ".. removing" if there is nothing to replacement.

I don't see a clear connection to prompt-issue #2498. That issue was not about outputs, it was about asking vs termination.

[dmpetrov]

This misleading warning makes this issue p1 or even p0 :(
Assigning a priority.

I don't see a clear connection to prompt-issue #2498. That issue was not about outputs, it was about asking vs termination.

@dmpetrov , we could terminate the checkout operation if there's a non-cached version of that file.

[dmpetrov]

@dmpetrov , we could terminate the checkout operation if there's a non-cached version of that file.

Right. But this is part of #2498, not the current one.