Add context about failing branches during error in used_cache()#5038
Add context about failing branches during error in used_cache()#5038courentin wants to merge 2 commits into
Conversation
|
This needs to be updated |
| cache.update(used_cache, suffix=suffix) | ||
| except DvcException as e: | ||
| logger.error(f"Cache for '{branch}' could not be collected") | ||
| if ignore_dvc_exceptions: |
There was a problem hiding this comment.
Seems like using --force would be intuitive here.
| if ignore_dvc_exceptions: | |
| if ignore_dvc_exceptions or force: |
WDYT?
There was a problem hiding this comment.
Do you suggest using force and ignore_dvc_exceptions or just force?
Indeed, --force seems more coherent with options across dvc commands
There was a problem hiding this comment.
Hum, --force is already used in the gc command "Force garbage collection - automatically agree to all prompts".
There was a problem hiding this comment.
@efiop Don't we want to differentiate --force where the behaviour is "Force garbage collection - automatically agree to all prompts" and --skip-failing-collect so that we do one without the other?
WDYT?
I'd like to merge this PR so that our team can clean their workspace :)
There was a problem hiding this comment.
@courentin Do you have a scenario in which those could be used separately?
There was a problem hiding this comment.
I'd say if we want to ignore failing commits but not agree to all prompts. But to be honest I'm fine with using only --force, I let you decide what's best for the product ;)
efiop
left a comment
There was a problem hiding this comment.
Thank you for the PR! 🙏
Ideally, this should skip particular malformed dvcfiles and not the whole branch. We have similar requests for metrics/params/plots parsing too. But this would be a great start too.
|
Yes indeed, not sure how to do it however |
There's not an easy way right now for this. I'd imagine something like the following, as stage collection error can be handled using a callback: def func(path, exc):
print(f"Collecting '{path}' failed and was skipped.")
with repo.handle_error("stage_collection", func):
stages = repo.stagesSomething like that should work for the CLI usage, but as an API, it might not as we may have collected the stages before running |
|
@skshetry Maybe just something generic and explicit like a |
|
Do you mind if after finding a good wording for this option, I try to merge this PR as it is and we can discuss a more general approach in an issue or discussion? |
We don't expect you to handle this right away. This was just us discussing a long-term solution, sorry for the noise here. It'd be great if you could respond to #5037 (comment) and #5066, that'd help us prioritize (if those features would be helpful for you). Thanks a lot for the contributions, @courentin. |
|
Closing as outdated. @courentin Thank you for your patience 🙏 , we've readjusted error handling for our metrics/plots/etc, and brancher is on our todo list for the near future. So please stay tuned. |
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
This PR is an implementation of #5037.
It is my first contribution to the dvc project, feel free to challenge it.