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 format for the dvc version command #3499 #4114

Merged
merged 11 commits into from
Jul 28, 2020
36 changes: 20 additions & 16 deletions dvc/command/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from dvc.exceptions import DvcException, NotDvcRepoError
from dvc.scm.base import SCMError
from dvc.system import System
from dvc.utils import is_binary, relpath
from dvc.utils.__init__ import relpath
skshetry marked this conversation as resolved.
Show resolved Hide resolved
from dvc.utils.pkg import PKG
from dvc.version import __version__

Expand All @@ -29,11 +29,10 @@ def run(self):

info = [
f"DVC version: {__version__}",
f"Python version: {platform.python_version()}",
f"Platform: {platform.platform()}",
f"Binary: {is_binary()}",
f"Package: {PKG}",
skshetry marked this conversation as resolved.
Show resolved Hide resolved
f"Supported remotes: {self.get_supported_remotes()}",
"--------------------------------- \n",
efiop marked this conversation as resolved.
Show resolved Hide resolved
f"Build Info: Python {platform.python_version()} on "
f"{platform.platform()} installed via {PKG}",
f"Supports: {self.get_supported_remotes()}",
]

try:
Expand All @@ -46,13 +45,11 @@ def run(self):
# `dvc config cache.shared group`.
if os.path.exists(repo.cache.local.cache_dir):
info.append(
"Cache: {}".format(self.get_linktype_support_info(repo))
"Cache types: " + self.get_linktype_support_info(repo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use {} formated string as above with just the addtion of Cache types:.

)
if psutil:
fs_type = self.get_fs_type(repo.cache.local.cache_dir)
info.append(
f"Filesystem type (cache directory): {fs_type}"
)
info.append(f"Directory cache: {fs_type}")
pmrowla marked this conversation as resolved.
Show resolved Hide resolved
else:
logger.warning(
"Unable to detect supported link types, as cache "
Expand All @@ -72,15 +69,15 @@ def run(self):

if psutil:
fs_root = self.get_fs_type(os.path.abspath(root_directory))
info.append(f"Filesystem type (workspace): {fs_root}")
info.append(f"Workspace with {fs_root}")

logger.info("\n".join(info))
return 0

@staticmethod
def get_fs_type(path):
partition = {
pathlib.Path(part.mountpoint): (part.fstype, part.device)
pathlib.Path(part.mountpoint): (part.fstype + " on " + part.device)
for part in psutil.disk_partitions(all=True)
}

Expand Down Expand Up @@ -115,10 +112,12 @@ def get_linktype_support_info(repo):
os.unlink(dst)
except DvcException:
status = "not supported"
cache.append(f"{name} - {status}")

if status == "supported":
cache.append(name)
os.remove(src)

return ", ".join(cache)
return "/".join(cache)

@staticmethod
def get_supported_remotes():
Expand All @@ -129,7 +128,12 @@ def get_supported_remotes():
if not tree_cls.get_missing_deps():
supported_remotes.append(tree_cls.scheme)

return ", ".join(supported_remotes)
if len(supported_remotes) == len(TREES):
return "All remotes"
elif len(supported_remotes) == 1:
return supported_remotes
else:
return ", ".join(supported_remotes)
pmrowla marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@skshetry skshetry Jun 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sahilbhosale63, the plan was to make Supports use both remotes and cache types. But, as we didnot go through that, I think it's okay to not go "All remotes" route (i.e what was before with comma separated list of remotes). But, it's your call.



def _get_dvc_repo_info(repo):
Expand All @@ -139,7 +143,7 @@ def _get_dvc_repo_info(repo):
if repo.root_dir != repo.scm.root_dir:
return "dvc (subdir), git"

return "dvc, git"
return "dvc + git"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make this uniform, either we should make it all commas or all plus-es. Could just use comma for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for cache types which seems to use / right now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for commas (since / is a path separator)
+1 to check other occurrences



def add_parser(subparsers, parent_parser):
Expand Down