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

Consider throwing exception/logging warning on dvc init --no-scm in git repo #2472

Closed
pared opened this issue Sep 6, 2019 · 8 comments
Closed
Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important

Comments

@pared
Copy link
Contributor

pared commented Sep 6, 2019

Currently DVC will silently ignore --no-scm on dvc init in git repository and use git as SCM.

That is not a big problem, but can make tests inconsistent:

def test1(dvc_repo, git)

and

def test2(git, dvc_repo)

will behave differently, as in second case dvc_repo.scm is Git instance and in first: NoSCM

@efiop efiop added enhancement Enhances DVC p2-medium Medium priority, should be done, but less important labels Sep 7, 2019
@jorgeorpinel
Copy link
Contributor

I ➕1 this.

If a user accidentally dvc init --no-scms in a Git repo, they may end up tracking data both with Git and DVC.

@jorgeorpinel
Copy link
Contributor

We could even automate detecting whether there's a Git repo underneath and not offer this option, merging this issue with #2901.

@efiop
Copy link
Contributor

efiop commented Feb 26, 2020

This is no longer the case, as no-scm is now explicit, so sequential dvc runs won't use git automatically.

@jorgeorpinel dvc init --no-scm doesn't look like an accidental thing, it is a very explicit command. If someone is running it, he better know what he is doing.

Closing this ticket, since the original issue is no longer relevant. Please feel free to create a new one or reopen if my reasoning about --no-scm is not convincing.

@efiop efiop closed this as completed Feb 26, 2020
@jorgeorpinel
Copy link
Contributor

What do you think about automating Git detection so the --no-scm option is not even needed @pared, @efiop, @shcheklein? Is it worth opening its own issue? Thanks

@efiop
Copy link
Contributor

efiop commented Feb 26, 2020

@jorgeorpinel So if no git is detected we will assume no-scm? If so, that is error-prone.

@jorgeorpinel
Copy link
Contributor

Yes, that's my suggestion. I'm trying to think of how that would be error prone but I'm not seeing it:

If the directory is Git-tracked, why would you use dvc init --no-scm? That would be a mistake (as we discussed in #2472 (comment) and #2472 (comment) above) so actually not having the explicit option seems like its LESS error-prone from this PoV.

If it's not tracked by Git, you HAVE to use --no-scm, or dvc init fails.

@efiop
Copy link
Contributor

efiop commented Feb 28, 2020

@jorgeorpinel Because it is an explicit action and you might want to --no-scm in a git repo if you don't want dvc to mess around with your git. We've used it previously for some debugging purpuses too. Yes, it is a very specific advanced feature that is not popular, but it has its own explicit flag that is unlikely to be used accidentally. Let's just leave it as is, it doesn't harm anything. 🙂

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Feb 28, 2020

you might want to --no-scm in a git repo if you don't want dvc to mess around with your git. We've used it previously...

OK if this is an actual use case then sure, keeping it makes sense.

But keeping something unnecessary because it doesn't seem to hurt would not be a strong argument IMO, I'd even argue this can hurt user experience compared to making it implicit/invisible.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important
Projects
None yet
Development

No branches or pull requests

4 participants