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

forbid using subrepo as a target #3444

Closed
pared opened this issue Mar 5, 2020 · 2 comments · Fixed by #3876
Closed

forbid using subrepo as a target #3444

pared opened this issue Mar 5, 2020 · 2 comments · Fixed by #3876
Assignees
Labels
bug Did we break something? enhancement Enhances DVC p1-important Important, aka current backlog of things to do research

Comments

@pared
Copy link
Contributor

pared commented Mar 5, 2020

Please provide information about your setup
DVC version(i.e. dvc --version), Platform and method of installation (pip, homebrew, pkg Mac, exe (Windows), DEB(Linux), RPM(Linux))

After introducing support for multiple DVC roots in #3257, in case of following setup:

.
├── .dvc
├── .git
└── subrepo
    ├── data
    ├── data.dvc
    ├── .dvc
    └── .gitignore

Running command dvc push/pull/fetch subrepo/data.dvc will have some unexpected results.
Eg. push will push to remote stored in parent repo.
The reason for that is that we are not resolving Repo basing on target, but cwd.
To solve this, we should either forbid initializing Repo under other Repo, or implement dynamic resolving of target paths.
Other commands that need to be checked for this issue:
destroy, remove, move, repro, status, import, get, root, update, pipeline.

As simply forbidding seems like easier way, we will probably need to solve this problem anyway, to support import and get.

Related to iterative/dvc.org#1022

@triage-new-issues triage-new-issues bot added the triage Needs to be triaged label Mar 5, 2020
@pared pared changed the title Handle targeted pull/push/fetch from parent DVC repository. Handle targeted commands from parent DVC repository. Mar 5, 2020
@efiop efiop added enhancement Enhances DVC p1-important Important, aka current backlog of things to do labels Mar 10, 2020
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Mar 10, 2020
@efiop efiop added the bug Did we break something? label Mar 10, 2020
@efiop
Copy link
Member

efiop commented Mar 31, 2020

Ok, so discussed with @pmrowla and @pared that it makes sense to forbid it. If someone asks for this in the future, we could introduce --subrepos or smth like that for push/pull/etc.

@efiop efiop added the research label Mar 31, 2020
@efiop efiop added this to To do in DVC 7 - 21 April 2020 via automation Mar 31, 2020
@efiop efiop removed this from To do in DVC 7 - 21 April 2020 Apr 21, 2020
@efiop efiop added this to To do in DVC Sprint 21 Apr - 5 May 2019 via automation Apr 21, 2020
@efiop efiop added this to To do in DVC Sprint 5 - 19 May 2019 via automation May 5, 2020
@efiop efiop removed this from To do in DVC Sprint 21 Apr - 5 May 2019 May 5, 2020
@efiop efiop removed this from To do in DVC Sprint 5 - 19 May 2019 May 19, 2020
@efiop efiop added this to To do in DVC Sprint 19 May - 2 June 2019 via automation May 19, 2020
@efiop efiop changed the title Handle targeted commands from parent DVC repository. forbid using subrepo as a target May 19, 2020
@efiop
Copy link
Member

efiop commented May 19, 2020

The reason it does't throw an error right now is https://github.com/iterative/dvc/blob/1.0.0a1/dvc/repo/__init__.py#L210 . It opens dvcfile directly without consulting with the clean tree.

As discovered by @skshetry , repo.tree doesn't use dvcignore when checking if something exists (only uses it when walk()ing). So we need to revisit CleanTree to fix that.

@pmrowla pmrowla self-assigned this May 26, 2020
@pmrowla pmrowla moved this from To do to In progress in DVC Sprint 19 May - 2 June 2019 May 26, 2020
@pmrowla pmrowla moved this from In progress to Review in progress in DVC Sprint 19 May - 2 June 2019 May 26, 2020
DVC Sprint 19 May - 2 June 2019 automation moved this from Review in progress to Done May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Did we break something? enhancement Enhances DVC p1-important Important, aka current backlog of things to do research
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants