Repository navigation
fix(push): report a bad target instead of a traceback (0.25.1) - #187
Merged
Merged
Conversation
`dt push myremote` -- the `-r` left off -- read the remote name as a target and died in DVC's collection with a traceback. The diagnosis was in there, but you had to read it out of a stack. Two parts. `utils.collect_tracked_entries` now catches DVC's target-resolution exceptions and raises `TargetNotFoundError` naming the target, which covers every caller of that helper, not just push. On top of that, push says what the mistake probably was: if the target is a configured remote name, it suggests `-r`. The plain push path shells out to `dvc push`, so it gets a pre-flight check rather than an exception to catch. That check refuses a target only when it both cannot resolve and names a remote, so nothing DVC would have accepted is turned away. Also fixes target extraction, which treated an option's value as a target: `dt push --dry --jobs 8` used to fail on target `8`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dt push myremote-- the-rleft off -- read the remote name as a target and died inside DVC's collection with a traceback. The diagnosis was in there, but you had to read it out of a stack.Before:
After, in every mode -- plain,
--dry,--workers:Where the fix sits
The traceback is fixed once, in
utils.collect_tracked_entries. That helper is the single place we call DVC's_collect_indexesdirectly, so catching DVC's target-resolution exceptions there (NoOutputOrStageError,OutputNotFoundError,PathMissingError,StageFileDoesNotExistError,StageNotFound) and re-raisingTargetNotFoundErrorcoversdt cachetoo, not just push. The new exception carries the offending targets so each caller can add its own hint.The hint is push's, because only push knows about
-r.explain_bad_target()checks the target against the configured remote names and suggests-r NAMEwhen it matches; otherwise it just says what a target is.The plain path needed a different mechanism. Without
--workers,dt pushshells out todvc push, so there is no exception of ours to catch -- only DVC's stderr. Rather than pattern-match that output,check_targets()runs first. It refuses a target only when it both cannot resolve and names a configured remote, which keeps it from turning away anything DVC would have accepted.Why the resolvability check is permissive
unresolvable_targets()accepts more than "a path that exists": a.dvcfile, a stage name indvc.yaml,dvc.yaml:stage, or a tracked parent directory all count.That last one is not theoretical. Checked against real DVC:
A stricter "does this path exist" test would have rejected a valid push. The cost of erring this way is that a mistyped non-remote target in plain
dt pushstill gets DVC's own message rather than ours -- no traceback either way, and DVC's message is clear.Also fixed
Target extraction was
[arg for arg in ctx.args if not arg.startswith('-')], which takes an option's value for a target --dt push --dry --jobs 8failed on target8.extract_targets()now skips the value of a value-taking option.Tests
New coverage for the conversion in
utils, the resolvability rules (including the not-checked-out cases above),extract_targets,explain_bad_target, andcheck_targets. Full unit suite on a compute node: 2408 passed, 2 skipped.🤖 Generated with Claude Code