Make the CLI version update a no-op; download skills from the Skills/ root - #277
Closed
xsh310 wants to merge 2 commits into
Closed
Make the CLI version update a no-op; download skills from the Skills/ root#277xsh310 wants to merge 2 commits into
xsh310 wants to merge 2 commits into
Conversation
ucode used to read `databricks --version` and, when it was below MIN_DATABRICKS_CLI_VERSION, shell out to brew/curl/wget (with sudo on Linux) to replace the user's install. Silently swapping a CLI the user manages themselves -- including a locally built one -- is more disruptive than letting a feature fail with the CLI's own error message. `ensure_databricks_cli_version()` is now a no-op, and the bootstrap path (renamed `install_databricks_cli` -> `ensure_databricks_cli`) only checks PATH, raising with install instructions instead of installing. Drops the installer helper, the version parser, and the min-version constant; the README now lists the CLI as a prerequisite. Co-authored-by: Isaac
…mes/`
Skill bundle content is served under `Skills/{cat}/{sch}/{leaf}/...`, so
`configure skills` was reading a `Volumes/` path that does not hold the
bundle. Both the recursive directory walk and the per-file fetch move
over; they have to agree, since relative paths are produced by stripping
the prefix off the absolute paths the listing returns.
Adds SKILL_FILES_ROOT so the root is stated once, and a test pinning the
directory-listing URL -- previously only the file-fetch URL was asserted,
so a half-applied change would have gone unnoticed.
Co-authored-by: Isaac
Collaborator
Author
|
Superseded by #278 (branch renamed to |
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.
Two independent changes to how
ucodetalks to Databricks.1. Make the Databricks CLI version update a no-op
Why
install_databricks_cli()used to readdatabricks --versionand, whenever it was belowMIN_DATABRICKS_CLI_VERSION(1.0.0), shell out to replace the install:brew upgrade databricks/tap/databrickson macOS, orcurl | sudo sh/wget | sudo shon Linux, orirm ... | iexon Windows.That silently swaps out a CLI the user manages themselves, including a locally built one, and does it with
sudoon Linux. Failing a feature with the CLI's own error message is less disruptive than mutating the developer's toolchain out from under them.What changed
ensure_databricks_cli_version()is now a no-op. It is kept as a seam so callers do not need to know whether a version policy exists.install_databricks_cli()is renamedensure_databricks_cli(). It only checksPATHand raises with install instructions whendatabricksis missing; it never installs._run_databricks_cli_installer(),_parse_databricks_cli_version(),MIN_DATABRICKS_CLI_VERSION, and the twosetup-cliinstall URLs.install_ai_tools()that claimed the CLI version was already guaranteed.Behavior change
A user with no
databricksonPATHpreviously got an automatic install; they now get an actionable error pointing at the install docs:A user on an old CLI previously got an automatic upgrade; they now proceed and, if they hit a subcommand their CLI lacks (for example
databricks aitools), see the CLI's own error.install_ai_tools()only warns on failure, so that path degrades rather than breaks.2. Download skill bundles from the Files API
Skills/rootWhy
configure skillsfetched bundle content fromVolumes/{catalog}/{schema}/{leaf}/..., but skill bundles are served under the Files API'sSkills/root. TheVolumes/path does not hold the bundle.What changed
fetch_skill_file()now requestshttps://{hostname}/api/2.0/fs/files/Skills/{catalog}/{schema}/{leaf}/{relative_path}.list_skill_files()walksGET /api/2.0/fs/directories/Skills/...and strips the matching/Skills/...prefix.Both halves have to move together: relative paths are produced by stripping the prefix off the absolute paths the listing returns, so a half-applied change yields unstripped paths rather than a clean failure. Added
SKILL_FILES_ROOTso the root is stated once.Testing
uv run ruff check .passes. Full non-e2e suite passes: 1216 tests.TestEnsureDatabricksCliVersionrewritten to pin the no-op contract: never raises for a new, old, or unparseable version, and shells out to nothing.TestEnsureDatabricksClicovers thePATHhit, the actionable raise on a miss, and that no install is ever attempted.test_walks_the_skills_root_not_volumespins the directory-listing URL. Previously only the file-fetch URL was asserted, so a half-applied prefix change would have gone unnoticed.Also smoke-tested the built tool from this branch (
uv tool install --reinstall "git+https://github.com/xsh310/ucode@noop-cli-version-update") and confirmed the missing-CLI error above appears withdatabricksoffPATH.E2E tests were not run: they need a live workspace. The
Skills/download path in particular is covered only by unit tests here, so it is worth exercisingucode configure skills --location <cat>.<sch>against a real workspace before merge.This pull request and its description were written by Isaac.