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

import/get: improve message when we try to re-import an artifact #2710

Closed
shcheklein opened this issue Nov 1, 2019 · 11 comments
Closed

import/get: improve message when we try to re-import an artifact #2710

shcheklein opened this issue Nov 1, 2019 · 11 comments
Labels
good first issue ui user interface / interaction

Comments

@shcheklein
Copy link
Member

shcheklein commented Nov 1, 2019

We need to check (and fail gracefully?) if someone tries to do import/get of an output from the imported stage.

Example:

mkdir test-import
cd test-import
git init
dvc init
dvc import https://github.com/iterative/example-get-started model.pkl
dvc remote add -d myremote /tmp/storage
git commit -a -m "add remote"
cd ..
mkdir test-recursive-import
git init
dvc init
dvc import ../test-import/ model.pkl

outputs:

Importing 'model.pkl (../test-import/)' -> 'model.pkl'
WARNING: Some of the cache files do not exist neither locally nor on remote. Missing cache files:
name: ../../../../private/var/folders/_1/dxrf7_f15sn4r01jvqvr5b6h0000gn/T/tmprcut42dgdvc-erepo/model.pkl, md5: 662eb7f64216d9c2c1088d0a5e2c6951
WARNING: Cache '662eb7f64216d9c2c1088d0a5e2c6951' not found. File 'model.pkl' won't be created.
ERROR: failed to import 'model.pkl' from '../test-import/'. - output 'model.pkl' does not exist

Having any troubles? Hit us up at https://dvc.org/support, we are always happy to help!

which is ugly and technically correct but can improved to warn that we are trying to re-import imported output which will always fail since we don't cache imports.

@ghost
Copy link

ghost commented Nov 1, 2019

@shcheklein , do you have an example?

@shcheklein
Copy link
Member Author

@MrOutis will do an example 👍 , should be quick and easy

@shcheklein

This comment has been minimized.

@shcheklein

This comment has been minimized.

@shcheklein shcheklein changed the title import/get: check how the behave and handle properly on imported artifacts import/get: improve message when we try to re-import an artifact Nov 1, 2019
@shcheklein shcheklein added ui user interface / interaction and removed research labels Nov 1, 2019
@kiranvinodcgs
Copy link

I would like to work on this issue

@kurianbenoy
Copy link
Contributor

Can't reproduce this issue @shcheklein

@efiop
Copy link
Contributor

efiop commented Nov 3, 2019

@shcheklein We could totally import recursive outs as well, it just requires some refactoring. But let's forbid it for now, until we get a request for that.

@kiranvinodcgs Sure! So to clarify, to fix this you need to put a check here https://github.com/iterative/dvc/blob/0.66.3/dvc/dependency/repo.py#L75 , that would look something like:

if out.stage.is_repo_import:
    raise DvcException("Recursive imports are currently unsupported.")

@efiop
Copy link
Contributor

efiop commented Nov 3, 2019

@kurianbenoy Indeed, the script provided by @shcheklein doesn't work. Here is a modified script:

#!/bin/bash
set -e
set -x

rm -rf test-import
mkdir test-import
pushd test-import
git init
dvc init
dvc import https://github.com/iterative/example-get-started model.pkl
dvc remote add -d myremote /tmp/storage
git add .
git commit -a -m "add remote"
popd

rm -rf test-recursive-import
mkdir test-recursive-import
cd test-recursive-import
git init
dvc init
dvc import ../test-import/ model.pkl

@shcheklein
Copy link
Member Author

@kurianbenoy @efiop sorry, guys! 🙏 will be more careful next time!

@Suor
Copy link
Contributor

Suor commented Nov 20, 2019

If we are to push a model registry use case and to lesser extend a data registry one we might want to make it work actually.

@dberenbaum
Copy link
Collaborator

Closing as irrelevant since we now support chained imports. Feel free to reopen if I misunderstood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue ui user interface / interaction
Projects
None yet
Development

No branches or pull requests

6 participants