Consolidate installer pre-flight checks in preflight.sh#211
Open
mh-avant wants to merge 1 commit into
Open
Conversation
Move shared color vars, status helpers, and the tool-prerequisite check into preflight.sh so install.sh and deploy.sh share one implementation. Rewrite _preflight_check_tools as collect-all (gather every missing tool and version issue, report them together, exit once) instead of failing on the first miss. Yellow-warn rather than green-check when the Databricks CLI version cannot be parsed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
|
@mh-avant I know you're a first time contributor, but wanted to find some time to chat with you about this PR |
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
2 participants
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.
Summary
scripts/preflight.shis now the single source of truth for shared shell helpers (color vars,_info/_ok/_warn/_error/_header) and the tool-prerequisite check.install.shsources them instead of redefining them._preflight_check_toolsis rewritten as collect-all: it gathers every missing tool and every version issue (Databricks CLI minimum 0.297.2, Node.js engine support) and reports them together before exiting once — no more fix-one-tool-rerun loops.databricks --versioncannot be parsed, the function now yellow-warns instead of green-checking, so the output doesn't misrepresent "we don't know" as "good."install.shStep 1 collapses from ~55 lines of bespoke per-tool checks to a single_preflight_check_toolscall.deploy.shis unchanged and inherits the new behavior automatically.Why
The two scripts had near-duplicate tool-presence/version checks with different output styling and slightly different logic. The previous behavior exited on the first missing tool, which is poor UX. Deploy-time checks remain important because
deploy.shruns many times (--updatemode) after the one-timeinstall.sh— tools, network, and auth state can change between runs.Test plan
bash -n scripts/install.sh scripts/preflight.sh scripts/deploy.shpassesbash -c 'source scripts/preflight.sh && _preflight_check_tools'reports all five tools correctly with colored outputMISSING,DB_VERSION,NODE_VERSION,PY_VERSION,_check_databricks_cli_version)./scripts/install.shend-to-end against a real workspace (reviewer)./scripts/deploy.sh --updateagainst an existing app (reviewer)🤖 Generated with Claude Code