Repository navigation
feat(auth): --repo for dt auth setup (0.20.0) - #177
Merged
Merged
Conversation
`remote.root` is a list key -- `dt config add remote.root <path>` promotes
a scalar to a list on first use, and `dt/remote.py` has always read it
through `cfg.get_str_list`. Endpoint discovery was the one consumer still
treating it as a plain string, so `Path(remote_root)` raised
TypeError: expected str, bytes or os.PathLike object, not list
as soon as a second root was configured. That crashed `dt auth list`,
`dt auth check` and `dt auth setup` outright, with a traceback rather than
an error -- and the site config here configures eight roots, so all three
were broken for everyone using it.
Go through `remote_mod.remote_roots()`, the shared accessor. A project's
remote lives under one of the roots, so offer the roots that actually hold
it, falling back to the first -- which is where `dt remote init` would
create it -- when none do.
Co-Authored-By: Claude <noreply@anthropic.com>
`dt auth check --repo` and `dt auth list --repo` could already inspect a repository you had not cloned; `dt auth setup` could not, so there was no way to *acquire* the access those commands told you that you lacked without first cloning. That is the wrong shape for `dt get`, whose whole point is fetching data without a tracked working copy. `--repo` shallow-clones the repo to a temp dir purely to read its config, so it runs from anywhere -- including a directory that is not a git repo. Credentials need scoping to match. `install_credentials()` with no argument discovers repos from the *current directory*, which under `--repo` is a different project entirely, so the secrets fetched would be the wrong ones. `_s3_secret_names` maps each discovered S3 endpoint back to the repo that owns it instead: a top-level remote belongs to the named repo, a nested one to the import source above it. A repo whose data comes from an import needs the source repo's credentials too, or the fetch fails on exactly the paths the caller came for. Also make a missing SSH username explain itself. `click.prompt` aborts with an empty message on a closed stdin, which surfaced as a bare "SSH setup error:" -- no host, no cause, no remedy. Non-interactive runs are the norm for this flag, so say what is missing and how to supply it. Co-Authored-By: Claude <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.
Adds
--repotodt auth setup, mirroring the option ondt auth checkanddt auth list.Those two could already inspect a repository you had not cloned, but there was no way to acquire the access they told you that you lacked without cloning first. That is the wrong shape for
dt get, whose point is fetching data with no tracked working copy.dt auth setup --repo git@github.com:org/data-repo.git dt auth setup --repo data-repo # short name, via `owner`The repo is shallow-cloned to a temp dir purely to read its config, so this runs from anywhere — including a directory that is not a git repo.
Credentials had to be scoped, not just discovered
install_credentials()with no argument discovers repos from the current directory, which under--repois a different project entirely — it would fetch the wrong secrets._s3_secret_namesmaps each discovered S3 endpoint back to the repo that owns it: a top-level remote belongs to the named repo, a nested one to the import source above it.That second case matters. A repo whose data comes from an import needs the source repo's credentials too, or the fetch fails on exactly the paths the caller came for.
Also fixed: a crash that broke all three auth commands
Verifying this on real data immediately hit an unrelated, unhandled crash:
remote.rootis a list key —dt config add remote.root <path>promotes a scalar to a list, anddt/remote.pyhas always read it throughcfg.get_str_list. Endpoint discovery was the one consumer still treating it as a scalar. The site config here sets eight roots, sodt auth list,dt auth checkanddt auth setupwere all crashing outright with a traceback for everyone using it. It is a separate bug, but it blocked the feature, so it is the first commit rather than a separate PR.The fix goes through
remote_mod.remote_roots()and offers the roots that actually hold the project, falling back to the first — wheredt remote initwould create it — when none do. Forbcarc_wtsthat correctly surfaces two roots.Also: a missing SSH username no longer says nothing
click.promptaborts with an empty message on a closed stdin, which surfaced as a bareSSH setup error:— no host, no cause, no remedy. Non-interactive runs are the norm for this flag, so it now names the host and points at-u.Verification
tests/unit) — 20 new tests, no regressionsbcarc_wtscorrectly resolves to the single secretbcarc_wts, and a real CLI run against a no-S3 repo left~/.aws/credentialsuntouchedtests/integration/test_clone_cmd.pyhangs on network access to GitHub from the login node — confirmed identical onmain(4m0.015s) and this branch (4m0.167s), so it is pre-existing and unrelated.Closes nothing — no issue was filed for this.
🤖 Generated with Claude Code