Skip to content

Conversation

@nik123
Copy link
Contributor

@nik123 nik123 commented Aug 25, 2020

Fixes #4383 (again)

After PR #4398 it occured, that dvc status -c output is bloated and informs about "missing" files twice (took from #4398 (comment)):

$ dvc status -c -R .
WARNING: Some of the cache files do not exist neither locally nor on remote. Missing cache files:
name: data-in-cache2-not-in-remote.txt, md5: 599b9eea7a2c4f97be18c077d2df4789
	new:                data-in-cache-1-not-in-remote.txt
	missing:            data-in-cache-2-not-in-remote.txt

If missing is now an expected status, the WARNING is probably no longer needed. So it could just output something like:

$ dvc status -c  # Assumes default remote exists, otherwise -r myremote
new:                data-in-cache-1-not-in-remote.txt
missing:            data-in-cache2-not-in-remote.txt

The WARNING shouldn't shown for dvc status -c. However it should be shown for all other commands (fetch, gc, etc.). This PR implements such behavior.

P.R.: I didn't come up with anything better than adding log_missing=True param into dvc.data_cloud.DataCloud.status and passing it all the way down dvc.cache.local._make_status. It's ugly. If there any better way please inform me about it.

issue in dvc.or repo already exist: treeverse/dvc.org#1701

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

@pared pared self-requested a review August 25, 2020 19:37
@pared
Copy link
Contributor

pared commented Aug 26, 2020

@nik123 the change looks good. I agree we should probably avoid passing down the flag, though I do not see any alternative as of now. Maybe its time we revisit #2957

jobs=None,
show_checksums=False,
download=False,
log_missing=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

We've been thinking about revisit logging/info interface for a long time, since passing a chain of such flags is not the prettiest thing ever 🙁 But there is no simple better way for now, so this will work for now 👍

Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

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

Thank you @nik123 !

@efiop efiop merged commit 1d9acdc into treeverse:master Aug 26, 2020
@nik123 nik123 deleted the i4383-remove-missing-warning-in-status branch August 26, 2020 14:59
@skshetry skshetry added the enhancement Enhances DVC label Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhances DVC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

status: ignoring cache that is not in local cache nor in remote

4 participants