Stage: fix used_cache warning on no checksum#3365
Conversation
There was a problem hiding this comment.
This property should only return what is already computed, it shouldn't start computing it all over again.
See dvc/outpt/base.py:get_used_cache, where the issue rose. We check if not self.info there, while we should probably check if not self.checksum. Push shouldn't push uncommited changes, as it doesn't even have the cache for them in .dvc/cache.
90cebdc to
e4b4506
Compare
There was a problem hiding this comment.
| out=self, stage=self.stage.relpath | |
| out=self, stage=self.stage |
same as above 🙂
There was a problem hiding this comment.
AFAIK it will result in dvc commit 'Stage file.dvc' ;)
There was a problem hiding this comment.
@pared Oh, but then the upper warning is broken too, right?
There was a problem hiding this comment.
@efiop
I wouldn't say so: the upper message is pointing user to which stage particular output is associated to, while the lower one suggests command that can fix your situation. So in the upper one, its not crucial for stage to be printed just as relpath.
There was a problem hiding this comment.
Maybe let's narrow it down to get_used_cache unit test for output?
1215516 to
5a711e8
Compare
❗ Have you followed the guidelines in the Contributing to DVC list?
📖 Check this box if this PR does not require documentation updates, or if it does and you have created a separate PR in dvc.org with such updates (or at least opened an issue about it in that repo). Please link below to your PR (or issue) in the dvc.org repo.
❌ Have you checked DeepSource, CodeClimate, and other sanity checks below? We consider their findings recommendatory and don't expect everything to be addressed. Please review them carefully and fix those that actually improve code or fix bugs.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏