Dvc: Full path in dvc add error

[dmpetrov]

I have a stage file images.dvc.

$ echo "\nsome-changes-----\n" >> images.dvc
$ dvc add data/file.txt
$ dvc add data/file.txt
Adding 'data/file.txt' to 'data/.gitignore'.
Saving 'data/file.txt' to cache '.dvc/cache'.
Error: unexpected error - while scanning a simple key
  in "/Users/dmitry/src/dvc_roadmap/tmp/images.dvc", line 9, column 1     # <-- FULL PATH???
could not find expected ':'
  in "/Users/dmitry/src/dvc_roadmap/tmp/images.dvc", line 11, column 1     # <-- FULL PATH???

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

In general, DVC should operate only by local paths (like Git does). There might be some exceptions like - external dependencies.

[pared]

@dmpetrov I PR-ed change that results in our own exception in case of yeaml structure breach. Though I am not sure if I quite get this bug right, do you think that we should read images.dvc by its relative path?

[shcheklein]

@pared I think the issue was to use relative path instead of the full path in the exception:

/Users/dmitry/src/dvc_roadmap/tmp/images.dvc

--->

tmp/images.dvc

(assuming dvc_roadmap is a git/dvc repo)

I would check other places where we print path, file names, etc.

[dmpetrov]

@shcheklein is right. It is only about user experience. We should avoid output full paths when it is possible.

[efiop]

Fixed by https://github.com/iterative/dvc/pull/1687

[pared]

@efiop It seems that we should not close it yet, as @dmpetrov mentioned, there should be only relative path in error message.

[efiop]

@pared Ah, right. For some reason, I thought that you've used relpath there already. Re-opening.

[pared]

I created another PR. If we want to refactor all exceptions/log messages, I would suggest to prepare list of places where we write something to stderr/stdout and make a checklist out of it, instead of one change, since it might be related to editing quite few tests that expect full path messages.

[pared]

@dmpetrov @shcheklein @efiop What do you think?

[efiop]

@pared No need to bother with that right now. The PRs you have submitted do the job perfectly. We use relpath in most(if not all) other places for stages anyway. If we'll find something wrong, we'll fix it later. Thanks! 🙂