get: checking target path before downloading

[julsal]

Fixes #3673

• ❗ I have followed the Contributing to DVC checklist.

• 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

[efiop]

Hi @julsal !

Thanks for the PR!

By default force=False, so if the output already exists and it differs from the desired one, then we will prompt user for removal confirmation. E.g.

#!/bin/bash                                                                       
                                                                                  
set -e                                                                            
set -x                                                                            
                                                                                  
rm -rf mytest                                                                     
mkdir mytest                                                                      
cd mytest                                                                         
                                                                                  
echo data > data                                                                  
dvc get https://github.com/iterative/dataset-registry get-started/data.xml -o data # will prompt

So this whole change is not really needed 🙁 You probably got confused by running dvc get ... twice, in which case it won't prompt you simply because the files are identical, so it doesn't have to prompt.

[julsal]

Hi @efiop,

Thanks for the quick reply.

The change comes in the context of #3673, which shows that dvc asks for confirmation only after downloading the file. For this reason I introduced an earlier prompt. force=True comes to avoid another prompt.

Does it make sense when considering the issue?

[efiop]

@julsal Ah, I see. Indeed, there was a misunderstanding on my side. Thank you for pointing out!

Just to clarify, this is a good PR 👍 and counts as a pass for the application purposes, but we want to reconsider this from the tree arch perspective, which will require some more effort. So just to not waste your more of your time on this, we could close the PR for now.

[julsal]

@efiop I agree. A future deeper refactoring might find a more elegant solution. Nonetheless, if you find useful, I changed the PR to keep only the test cases for file replacement.

[efiop]

Thank you @julsal 🙏