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

version: redesign output #3499

Closed
efiop opened this issue Mar 17, 2020 · 21 comments · Fixed by #4114
Closed

version: redesign output #3499

efiop opened this issue Mar 17, 2020 · 21 comments · Fixed by #4114
Labels
enhancement Enhances DVC good first issue help wanted p3-nice-to-have It should be done this or next sprint ui user interface / interaction

Comments

@efiop
Copy link
Contributor

efiop commented Mar 17, 2020

Current dvc version output is ugly, we could definitely do better:

DVC version: 0.89.0+9e96dd
Python version: 3.7.0
Platform: Linux-4.15.0-88-generic-x86_64-with-debian-buster-sid
Binary: False
Package: None
Supported remotes: azure, gdrive, gs, hdfs, http, https, s3, ssh, oss
Cache: reflink - not supported, hardlink - supported, symlink - supported
Filesystem type (cache directory): ('ext4', '/dev/sda1')
Filesystem type (workspace): ('ext4', '/dev/sda1')

Related #3185

@triage-new-issues triage-new-issues bot added the triage Needs to be triaged label Mar 17, 2020
@efiop efiop added enhancement Enhances DVC good first issue help wanted p3-nice-to-have It should be done this or next sprint labels Mar 17, 2020
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Mar 17, 2020
@efiop efiop added triage Needs to be triaged ui user interface / interaction labels Mar 17, 2020
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Mar 17, 2020
@efiop
Copy link
Contributor Author

efiop commented Mar 17, 2020

maybe something like this yaml (btw need to look into possibly unifying analytics collection with this):

DVC: 0.89.0+9e96dd                                             
Python: 3.7.0                                                  
Platform: Linux-4.15.0-88-generic-x86_64-with-debian-buster-sid
Binary: False                                                  
Package: pip                                                   
Remotes:                                                       
  azure: supported                                             
  gdrive: supported                                            
  gs: supported                                                
  hdfs: supported                                              
  http: supported                                              
  https: supported                                             
  s3: not supported                                            
  ssh: supported                                               
  oss: supported                                               
Links:                                                         
  reflink: not supported                                       
  hardlink: supported                                          
  symlink: supported                                           
Cache:                                                         
  fs: ext4                                                     
  mnt: /dev/sda1                                               
Workspace:                                                     
  fs: ext4                                                     
  mnt: /dev/sda1                                               

easilly greppable and json-compatible.

Could also use some colors to highlight stuff

efiop added a commit to iterative/dvc.org that referenced this issue Mar 29, 2020
`dvc version` output was adjusted for urgent debugging purposes and I didn't
send a doc update because it is terribly ugly right now anyway. Instead, I've
created iterative/dvc#3499 to tackle that, but
@shcheklein reasonably requested a temporary doc update just for the
sake of it, so here we go.
shcheklein pushed a commit to iterative/dvc.org that referenced this issue Mar 29, 2020
`dvc version` output was adjusted for urgent debugging purposes and I didn't
send a doc update because it is terribly ugly right now anyway. Instead, I've
created iterative/dvc#3499 to tackle that, but
@shcheklein reasonably requested a temporary doc update just for the
sake of it, so here we go.
@sahilbhosale63
Copy link
Contributor

Hi, Is there anyone working on this issue. if not then can I work on this issue?

@efiop
Copy link
Contributor Author

efiop commented Jun 11, 2020

@sahilbhosale63 Sure! Let's start by discussing the output format in this ticket, so that we are on the same page and then, once we agree, we could proceed implementing it in the code. Thank you! 🙏

@sahilbhosale63
Copy link
Contributor

Yes, Let's start.

@sahilbhosale63
Copy link
Contributor

sahilbhosale63 commented Jun 18, 2020

Can you please provide me with the path to the file where this code resides?

@skshetry
Copy link
Member

Hi @sahilbhosale63, version command is here: dvc/command/version.py.

@skshetry
Copy link
Member

@sahilbhosale63, it'd be great to see example output before implementing. What do you have in mind?

Also, feel free to discuss this here and/or in chat on #dev-talk channel, we'll be happy to help you.

@sahilbhosale63
Copy link
Contributor

Currently I don't have any ideas in my mind. I think the output format suggested by @efiop will be the best fit. And surely I will try to modify the output format and according let you know if any idea looks good to me and after that we can have a discussion.

@efiop
Copy link
Contributor Author

efiop commented Jun 18, 2020

@sahilbhosale63 That was just a random suggestion, I admittedly didn't think about it too much. But that could be a good start and it definitely more informative than it is right now. If there are no other suggestions, please feel free to give it a shot.

@sahilbhosale63
Copy link
Contributor

Screenshot from 2020-06-23 16-41-21

This is the change which I have made. @efiop What do you think of it? Would you like to give any suggestions on this?

@efiop
Copy link
Contributor Author

efiop commented Jun 23, 2020

@sahilbhosale63 That looks good! Please feel free to submit a PR. Thank you! 🙏

@skshetry
Copy link
Member

@sahilbhosale63, @efiop, I think we should try making the output format human-readable rather than longer. For me, and the team, the important use case of dvc version is to help/debug other users' issues. Making it longer only makes it cumbersome to share/read.

How about something like this:

DVC version: 0.89.0+9e96dd
---------------------------------------

Build Info: Python3.7 on Linux, installed via pip
Supports: All remotes and cache types
Repo: dvc + git, workspace with 'ext4' on '/dev/sda1'
Cache: ext4 on '/dev/sda1'

There's a room for improvement of ^ for sure.

@efiop
Copy link
Contributor Author

efiop commented Jun 23, 2020

@skshetry Sure, that looks pretty good! @sahilbhosale63 would you be up for changing the output to that new format?

@sahilbhosale63
Copy link
Contributor

Sure @efiop, I will make the above change to a new format as suggested by @skshetry

@shcheklein
Copy link
Member

can we do something with that ugly warning? can we embed it into the Cache type: (no cache dir)?

@sahilbhosale63
Copy link
Contributor

Screenshot from 2020-06-24 12-21-06

Here, the "Supports" and "Cache" lines highlighted in red in the above img should come in a single line under "Supports"as suggested by @skshetry.

But the thing is some remotes or caches might not be supported in some systems. As in this case, reflink is not supported. So, here we can't just put the msg as "Supports: All remotes and cache types".

So what should be done here?

@skshetry
Copy link
Member

@sahilbhosale63, most of the times, dvc will either support all of the remotes or any one of them (two or three if there's already some dependencies installed but rare). And, http/https are always supported by default.
So, we could optimize these for usual scenarios and make it a bit ugly for rare scenarios.

Also, I'm fine with the following:

# Single remote:
Supports: gs remote, and hardlink/symlink cache types
# if it supports all remotes:
Supports: All remotes and reflink/hardlink/symlink cache types
# if it supports most of the remotes, but not all, we can make it a bit ugly as it is a rare scenario:
Supports: gs, azure, hdfs, azure, oss, ssh, gdrive remotes and reflink/hardlink/symlink cache types

As you can see, the last example is a bit lengthy. Also @sahilbhosale63, feel free to change the output format
wherever it does not make sense or if it could be improve further.

Thanks a lot for taking the task. 👍

@sahilbhosale63
Copy link
Contributor

Screenshot from 2020-06-24 16-15-08

I have made the required changes. The only think which I want to ask is there is a line break after the end of this line "Supports: All remotes" because of this line of code logger.info("\n".join(info))

The code adds a line break after every sentence but I don't want a line break after "Supports: All remotes" so that the output would look like this "Supports: All remotes and hardlink/ symlink cache types" in a one single line.

So is there anyway we can achieve that in python??

@shcheklein
Copy link
Member

@skshetry what should we do if we can't test for link type support?

I feel that we should not mix this information. Remotes are remotes, links are links - they are different, not related to each, so why do we show them in a single line?

sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jun 25, 2020
Redesigned the output format for the command: dvc version
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jun 25, 2020
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jun 26, 2020
Done some suggested fixes in code for the dvc command output format issue.
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jun 27, 2020
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jun 27, 2020
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jun 28, 2020
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jul 11, 2020
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jul 11, 2020
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jul 23, 2020
sahilbhosale63 added a commit to sahilbhosale63/dvc that referenced this issue Jul 23, 2020
efiop added a commit that referenced this issue Jul 28, 2020
)

* Version: Redesign output format #3499

Redesigned the output format for the command: dvc version

* version: reformat output format #3499

Done some suggested fixes in code for the dvc command output format issue.

* version: redesign output format #3499

* version: redesign output format for dvc command #3499

* version: redesign output format #3499

* version: redesign output format #3499

* Redesign output format for the dvc version command #3499

* Redesign output format for the dvc version command #3499

* Update dvc/command/version.py

Co-authored-by: Saugat Pachhai <[email protected]>

Co-authored-by: Ruslan Kuprieiev <[email protected]>
Co-authored-by: Saugat Pachhai <[email protected]>
@shcheklein
Copy link
Member

@efiop @sahilbhosale63 do we need to update docs?

@sahilbhosale63
Copy link
Contributor

@efiop @sahilbhosale63 do we need to update docs?

Yes @shcheklein, We have to update the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC good first issue help wanted p3-nice-to-have It should be done this or next sprint ui user interface / interaction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants