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

pull: not providing response on success #2667

Closed
bhavaniravi opened this issue Oct 24, 2019 · 16 comments
Closed

pull: not providing response on success #2667

bhavaniravi opened this issue Oct 24, 2019 · 16 comments
Assignees
Labels
enhancement Enhances DVC ui user interface / interaction

Comments

@bhavaniravi
Copy link

I was following the get started tutorial with local storage, followed the steps until dev pull - https://dvc.org/doc/get-started/retrieve-data.

When I do a dev pull and the data is fetched I don't get any response saying Sucess I have do a ls to verify whether the data folder has been created

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Oct 24, 2019

Thanks @bhavaniravi. I'm going to move your report over to the DVC core repo for handling.

We'll keep in mind the outcome of this ticket in the dvc.org documentation as well.

@jorgeorpinel jorgeorpinel transferred this issue from iterative/dvc.org Oct 24, 2019
@jorgeorpinel jorgeorpinel changed the title dvc pull not providing response on success pull: not providing response on success Oct 24, 2019
@jorgeorpinel jorgeorpinel added the enhancement Enhances DVC label Oct 24, 2019
@shcheklein shcheklein added bug Did we break something? ui user interface / interaction labels Oct 24, 2019
@shcheklein
Copy link
Member

Thanks, @bhavaniravi ! The same with dvc checkout actually in certain cases. We've removed too much from the output at some point and need to get some stuff back a little bit cc @casperdcl

@casperdcl casperdcl self-assigned this Oct 24, 2019
@efiop efiop removed the bug Did we break something? label Oct 24, 2019
@efiop
Copy link
Contributor

efiop commented Oct 24, 2019

I'll close this one in favor of #1035

@efiop efiop closed this as completed Oct 24, 2019
@shcheklein
Copy link
Member

I think #1035 was about dvc run commands, when you execute repro. For example, you can imagine you can just capture command outputs when you run repro and show only OK marks when something is done.

This one is about pull and checkout being completely silent. Even though they are doing something. Or they they say something like "Everything is up to date" at the end (while actually they changed the state). So, I would say the solution here should be writing a summary of changes for example (like git checkout), or at least saying a sentence like "Data has been updated".

@shcheklein shcheklein reopened this Oct 24, 2019
@efiop
Copy link
Contributor

efiop commented Oct 24, 2019

@shcheklein No, it was about other commands as well, I remember the discussion we've had. I see no point in keeping this issue opened when we have #1035

@shcheklein
Copy link
Member

@efiop kk. I recall it in a different way :) And probably it means that ticket is not specific enough, lacks some details (@bhavaniravi should have been able to find that ticket at least and upvote it, for example). I would keep this one if you want to keep only one.

Also, I think the approach from #1035 won't be solving the issue. It will be just printing "ok" every time which is not enough again to say what has just happened.

Also, printing "ok" for every DVC command is kinda cumbersome. I understand the value when we print it for repro stages (and hide output), I don't understand the value to show this every time.

@efiop
Copy link
Contributor

efiop commented Oct 24, 2019

@shcheklein I'm no saying we should print "OK", I just used that as an example there. I'm sure there is a better way to signal successful execution, but it will surely need to be applied to all commands. Or you think each command needs a special "ok" message?

@shcheklein
Copy link
Member

yep, that's the point. Take git - each commands writes some useful info about what has just happened, not just a generic "ok".

@efiop
Copy link
Contributor

efiop commented Oct 25, 2019

Well, we've established that git is not the best example 🙂 But I think that all of those messages need to be considered simultaneously to be in the same style, hence why I want to leave only #1035 and deal with specifics later. There is no value in having this issue opened with #1035 closed, as it talks about a more general problem.

@shcheklein
Copy link
Member

I mean, yes and no :) Like I mentioned I don't like #1035 because it's very confusing in terms of title, of what exactly should be solved there, it's not actionable (it's impossible to solve it reasonably for all commands at once, so it's better to split it anyways, or have sub-tickets). It lacks all the discussion here (and making related won't the issue) :). Also, it's fine to have one epic ticket and multiple checkboxes in it.

And, if you are still not sold :) May be it makes to repurpose this one to be more generic at this stage (because that one is just terrible :)).

@efiop
Copy link
Contributor

efiop commented Oct 25, 2019

So how #1035 or this one should look like in a general form? Name of #1035 seems fine, or do you have a better one in mind? Also, you think we should create checkboxes for every and each command? That seems strange. #1035 is worded in a generic form, talking about all dvc commands in the name of the ticket and providing (bad) example in the description.

@shcheklein
Copy link
Member

Well, as I mentioned I would just keep this one, not making it even general. I think the #1035 is ambiguous, does not provide any useful information, the action it suggests (the way you interpret it) only partially solves or does not solve at all the problem. I just don't get why would I need a generic "ok"-style output at all? How can it solve the problem here? I can just check the exit code easily for this to see was it "ok" or not, right? I think the better way is providing some meaningful output about the command outcome (like we do in dvc add now, and clearly we don't need anything else for dvc add unless I'm missing something?).

If it won't be about generating a generic "ok"-style message across all commands and actually come up with something on a command by command basis, how can we make that ticket actionable at all? We'll have to split it - the same way we are improving add and checkout now, right?

@casperdcl
Copy link
Contributor

could create a project and add both these issues (and other related future ones) to it

@efiop
Copy link
Contributor

efiop commented Oct 25, 2019

@shcheklein I think there is a miscommunication here. The issue says I.e. green "OK" or something better. and the key part is "something better", which is probably a per-command message. So I have to stress once again that I didn't mean that we should have a generic "ok" everywhere, I was just showing an example there to present the general idea.

Sure, every command needs to be revisited separataly as @casperdcl does, so in that sense, there is no point in #1035 as it generalizes a common issue with multiple commands, which would have to be resolved separately. Closing #1035 then, as it apparently doesn't bring any value.

@shcheklein shcheklein added p1-important Important, aka current backlog of things to do and removed p1-important Important, aka current backlog of things to do labels Oct 29, 2019
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Nov 22, 2019

So this is basically a duplicate of #1838?

@efiop
Copy link
Contributor

efiop commented Nov 23, 2019

Closing in favor of #1838

@efiop efiop closed this as completed Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC ui user interface / interaction
Projects
None yet
Development

No branches or pull requests

5 participants