Skip to content

Conversation

@girish946
Copy link
Contributor

comamnd 'dvc cache dir' now prints the current cache dir
when no cmd flags are provided.

Fixes #1998

Signed-off-by: Girish Joshi girish946@gmail.com

@girish946 girish946 force-pushed the 1998-dvc-cache-dir-print branch from dd469e8 to e2e9fb7 Compare September 24, 2020 11:38
…ided

tests: corrected the test for cache dir cmomand

comamnd 'dvc cache dir' now prints the current cache dir
when no cmd flags are provided and modified existing test for
the same.

Fixes treeverse#1998

Signed-off-by: Girish Joshi <girish946@gmail.com>
@girish946 girish946 force-pushed the 1998-dvc-cache-dir-print branch from e2e9fb7 to 022ab73 Compare September 25, 2020 05:17
@efiop efiop requested review from pmrowla and skshetry and removed request for pmrowla and skshetry September 28, 2020 12:38
Comment on lines 10 to 12
if not self.args.value:
print(self.config["cache"]["dir"])
return 0
Copy link
Contributor

@efiop efiop Sep 28, 2020

Choose a reason for hiding this comment

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

We have a similar logic in dvc config: https://github.com/iterative/dvc/blob/738dfd49cf403412b018677e62b0b68e85c80886/dvc/command/config.py#L19 , but it also checks for --unset and uses logger.info (our current convention). Seems like we could do the same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a similar logic in dvc config:

, but it also checks for --unset and uses logger.info (our current convention). Seems like we could do the same here.

done.

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.

Hi @girish946 !

Thank you for your patience! Looks good! Please see a comment above.

according to current convention in dvc/command/cache.py

Signed-off-by: Girish Joshi <girish946@gmail.com>
@girish946
Copy link
Contributor Author

Hi @efiop,
Thanks for the review.
I've made the change mentioned above.

@girish946 girish946 requested a review from efiop October 2, 2020 11:59
@efiop efiop merged commit 1d550f3 into treeverse:master Oct 12, 2020
@efiop
Copy link
Contributor

efiop commented Oct 12, 2020

Thank you @girish946 ! 🙏

@efiop efiop added the feature is a feature label Oct 12, 2020
@jorgeorpinel
Copy link
Contributor

Hi @girish946 would you like to also update the doc in https://dvc.org/doc/command-reference/cache/dir to reflect the change? (See https://dvc.org/doc/user-guide/contributing/docs) Np either way, just lmk please so we take care of it otherwise. Best

@girish946
Copy link
Contributor Author

Hi @jorgeorpinel
Thanks for reminding me about the documentation part that is remaining.
I'll update it in a day or two.

Best.

@jorgeorpinel
Copy link
Contributor

Sounds good, thanks @girish946

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature is a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

have dvc cache dir print the current cache dir when no cmd flags are used

3 participants