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

update: improve output when import stage is fixed to an unchanged rev? #2696

Closed
jorgeorpinel opened this issue Oct 30, 2019 · 9 comments
Closed
Labels
discussion requires active participation to reach a conclusion enhancement Enhances DVC good first issue p1-important Important, aka current backlog of things to do question I have a question? ui user interface / interaction

Comments

@jorgeorpinel
Copy link
Contributor

Moving from iterative/dvc.org#735 (comment)

$ dvc version
DVC version: 0.65.0
Python version: 3.7.4
Platform: Darwin-19.0.0-x86_64-i386-64bit
Binary: False

When you import a specific revision that doesn't change, for example a tag like:

$ dvc import --rev cats-dogs-v1 \
             git@github.com:iterative/dataset-registry.git \
             use-cases/cats-dogs
Importing 'use-cases/cats-dogs (git@github.com:iterative/dataset-registry.git)' -> 'cats-dogs'
Output 'cats-dogs' didn't change. Skipping saving.                                                                                                           
Saving information to 'cats-dogs.dvc'.
$ ls
total 8
drwxr-xr-x  3 usr  staff    96B Oct 22 14:05 cats-dogs/
-rw-r--r--  1 usr  staff   340B Oct 22 14:05 cats-dogs.dvc

And then you try to update, nothing changes because the rev never moves. However dvc update output doesn't really signal this, in fact it says "Saving information..." as if something happened:

$ dvc update cats-dogs.dvc
Saving information to 'cats-dogs.dvc'.

Although this is correct update behavior (as explained in iterative/dvc.org#735 (comment)), maybe the output could be different when it detects a fixed rev field in the DVC-file, and this rev hasn't changed. It could give an INFO message to suggest the user may have to re-import instead of updating. Something like:

$ dvc update cats-dogs.dvc
NOTE: The 'cats-dogs.dvc' import stage is fixed to revision 'cats-dogs-v1'
and this Git reference has not moved. You may want to re-import 'cats-dogs.dvc'
to un-fix it, instead of trying to update it.
@jorgeorpinel jorgeorpinel added enhancement Enhances DVC question I have a question? labels Oct 30, 2019
@shcheklein shcheklein added p1-important Important, aka current backlog of things to do ui user interface / interaction labels Oct 30, 2019
@efiop
Copy link
Contributor

efiop commented Nov 4, 2019

The Saving information part might be gone soon, as it is a bit redundant and will probably be replaced with a better summary. But the situation of branch/tag that hasn't moved does indeed need to be handled better. I like the idea of that note, but also wonder if we should error-out if trying to update something that is already at the latest version. Kinda like brew uprade dvc does. Just a thought.

@jorgeorpinel
Copy link
Contributor Author

wonder if we should error-out if trying to update something that is already at the latest version. Kinda like brew uprade dvc does.

Yeah good point. Maybe this is best indeed, to be extra explicit about what happened or did not happen (:

p.s. a shorter version of the note text, in case needed: "NOTE: The 'cats-dogs.dvc' import stage is fixed to Git revision 'cats-dogs-v1', which has not moved. See https://man.dvc.org/import for more details."

@shcheklein
Copy link
Member

I would say improving an output is a good first step (I though don't like NOTE prefix, it should be just a normal simple one sentence output) we can do.

@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Nov 4, 2019

  • Improving the output sounds like a good first issue! Would not close this ticket but doesn't hurt either.

@Suor
Copy link
Contributor

Suor commented Nov 5, 2019

I an against erroring out when nothing changed, this doesn't make sense to me and breaks scripting.

On the prefix, I would like to see HINT: there, because that is a hint actually, not something done or happened, but only some supposition.

@shcheklein
Copy link
Member

Agreed on both with @Suor . Probably would even avoid all upper case prefix at all. For a good message intention should be clear without providing additional prefixes. Or at least, make it just a regular normal part of the sentence - Hint, ....

@Suor
Copy link
Contributor

Suor commented Nov 5, 2019

I personally do like those uppercase prefixes, they optimize human text parsing. E.g. you just skip HINT of you know what you are doing or you read only WARNINGs, these are anchors, which help you navigate

@shcheklein
Copy link
Member

This is related, btw #2605. I don't mind HINT, let's try and see how does it look.

@pared pared added the discussion requires active participation to reach a conclusion label Nov 26, 2019
@dberenbaum
Copy link
Contributor

Current output signals that nothing changed: 'cats-dogs.dvc' didn't change, skipping. Closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion requires active participation to reach a conclusion enhancement Enhances DVC good first issue p1-important Important, aka current backlog of things to do question I have a question? ui user interface / interaction
Projects
None yet
Development

No branches or pull requests

6 participants