apps: git-back scaffolded apps when init runs inside a Git repo - #6406
apps: git-back scaffolded apps when init runs inside a Git repo#6406atreyadbrx wants to merge 2 commits into
Conversation
`databricks apps init` now detects the Git repository that will contain the new app and scaffolds a git-backed databricks.yml — git_repository (origin url + inferred provider) and git_source (current branch + repo-relative source_code_path) — instead of a plain source_code_path upload. Detection is best-effort and conservative: it falls back to source_code_path when there is no repo, no origin remote, an unrecognized provider host, a detached HEAD, or a destination outside the repo, so it never emits an unusable git block. Backward compatible with the AppKit template: older CLIs render the git fields empty and the template keeps source_code_path. Pairs with the AppKit template change in databricks/appkit#556. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: atreyadbrx <atreya.misra@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: atreyadbrx <atreya.misra@databricks.com>
Replace the commented git_repository/git_source example with a conditional that the Databricks CLI's `apps init` fills in when scaffolding inside a Git repository (databricks/cli#6406). When the CLI detects an origin remote it renders a real git_repository (url + provider) and git_source (branch + repo-relative source_code_path); otherwise the template keeps source_code_path, so older CLIs and non-repo scaffolds are unaffected. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: atreyadbrx <atreya.misra@databricks.com>
Approval status: pending
|
Integration test reportCommit: c9afa34
Top 3 slowest tests (at least 2 minutes):
|
|
Closing — changing direction per review feedback. Instead of detecting git at apps init/create time, git-backing will be an explicit opt-in migration for existing apps, driven by the databricks-apps skill (using the already-shipped apps update --git-* flags for the imperative path). The detection and gh-repo-create logic here will be salvaged into that migration skill. |
pkosiec
left a comment
pkosiec
left a comment
There was a problem hiding this comment.
Reviewed with the paired appkit#556. The core looks solid: no network call on local init, a correct Create vs Deploy split (git_repository to Create, git_source to Deploy with the nested repo stripped), exactly one of source_code_path or git_source so validation passes, and a conservative fallback to local wherever detection declines. Findings are non-blocking; inline comments cover the code.
P1: the backward compat claim is wrong.
- The description says an older CLI renders the git fields empty and keeps source_code_path.
- The opposite happens, and it is easy to reproduce. With no
gitkey, the new template's{{if .git.url}}fails hard ("nil pointer evaluating interface {}.url"). missingkey=zero only saves the first lookup, not the chained field. - Safety actually comes from cli-compat.json pinning each CLI version to a template tag, not from the template.
apps init --version lateston any pre-#6406 CLI clones appkit main and hits this once #556 lands there. - Please fix the wording.
FYI (no action): not bumping cli-compat.json here is correct. The compat bump happens later, after evals pass, and that is also what gates when the new template reaches users. Good sequencing.
P2: the unpushed deploy path is opaque.
- Git-backed deploy needs the branch committed and pushed first. Run
apps initthenapps deploybefore pushing and you get a raw server error with no hint. - This is the new default inside a repo. Consider detecting the missing ref and printing "commit and push first". The deploy path is outside this diff.
P2: private repos need a linked git credential.
- Deploy clones the repo on the workspace side. A private repo needs a linked git credential for the provider (git_repository has caller_credential_id). The scaffold sets only url and provider and cannot detect visibility, so a private repo with no linked credential fails at deploy, with no init-time hint.
- Question for the author: how is auth handled for private repos on deploy? Does it use the caller's linked credential on its own, or must something be set? Should init warn when git-backed is chosen but no credential is linked? This is inferred from the schema (caller_credential_id), not from PR code, since the clone is server side.
P2: add an acceptance test.
- Run
apps initin a fake git repo and golden file both the rendered git block and the no-git fallback. - It locks the url/provider/branch/sourceCodePath contract that P1 depends on. acceptance/apps/ already exists.
Questions on the end to end flow (init is interactive and non-interactive):
- No opt-out. Detection runs in both modes, with no flag (--source or --no-git) and no prompt.
- CI and non-interactive:
apps init --name xinside a repo silently produces git-backed config. Automation that expected a local upload changes behavior with no signal. Should there be an opt-out flag? - Interactive: the choice is never shown or confirmed, unlike other resources. Should it be surfaced?
- Determinism: the same command yields a different databricks.yml depending on whether the destination is in a repo. Intended for scripts?
- Detection keys off destDir, so --output-dir is honored, not CWD. Please confirm that is intended. An --output-dir into a different repo uses that repo's origin.
Minor:
- FetchRepositoryInfoAPI can nil deref on a Databricks runtime with a dest under /Workspace/ and a nil client. Local runs are fine.
- A line noting the deployed branch is pinned to the current branch at init time would help.
| // the bundle deploys from the repo/ref instead of uploading local files. | ||
| // Falls back to a plain source_code_path when there is no repo to point at. | ||
| gitSource := detectGitScaffoldSource(ctx, destDir, cmdctx.WorkspaceClient(ctx)) | ||
| if gitSource.active() { |
There was a problem hiding this comment.
The mode switch is only logged at debug level, so it is invisible by default. Consider a visible success line ("Configured git-backed deploy from (), branch "), and a short reason when a repo is present but git-backing is declined (unknown provider, detached HEAD, no origin, dest outside repo). Today the user cannot tell which mode they got, or why.
| return "bitbucketCloud" | ||
| case "dev.azure.com": | ||
| return "azureDevOpsServices" | ||
| default: |
There was a problem hiding this comment.
Azure DevOps SSH remotes are dropped silently. ssh.dev.azure.com (e.g. git@ssh.dev.azure.com:v3/org/proj/repo) maps to no provider, so it falls back to local, and the test locks that in. SSH is a common Azure clone method. Consider mapping ssh.dev.azure.com to azureDevOpsServices and rewriting v3/org/proj/repo to the dev.azure.com/org/proj/_git/repo https form, with a test.
| if provider == "" { | ||
| return "", "" | ||
| } | ||
| path = strings.Trim(strings.TrimSuffix(path, ".git"), "/") |
There was a problem hiding this comment.
Trim order mishandles a trailing slash after .git. For "o/r.git/", TrimSuffix(".git") does nothing (the string ends in "/"), then Trim("/") gives "o/r.git", so .git stays in the URL. Trim "/" first, then TrimSuffix(".git"). Rare input, cheap to fix.
| if path == "" { | ||
| return "", "" | ||
| } | ||
| return "https://" + host + "/" + path, provider |
There was a problem hiding this comment.
Host case leaks into the URL. "GitHub.com" matches the provider (the match is case insensitive), but the emitted URL keeps "GitHub.com". Git hosts are case insensitive, so this is likely harmless. Lowercasing the host avoids any host vs provider mismatch on the backend.
| } | ||
|
|
||
| sourcePath := "./" | ||
| if rel, relErr := filepath.Rel(info.WorktreeRoot, absDest); relErr == nil { |
There was a problem hiding this comment.
On filepath.Rel error (e.g. cross volume on Windows) the code keeps sourcePath "./" and still returns an active source at the repo root. Unreachable today, since WorktreeRoot is always an ancestor of absDest, so it is benign. return gitScaffoldSource{} on relErr is safer if that ever changes.
3 participants
Changes
databricks apps initnow detects the Git repository that will contain the new app and scaffolds a git-backeddatabricks.yml— agit_repository(origin URL + inferred provider) andgit_source(current branch + repo-relativesource_code_path) — instead of a plainsource_code_pathupload.cmd/apps/gitsource.go:detectGitScaffoldSourcewalks up from the scaffold destination viagit.FetchRepositoryInfo, normalizes the origin remote to an https URL, and maps the host to a Databricks git provider (gitHub,gitLab,bitbucketCloud,azureDevOpsServices).cmd/apps/init.go: the detected fields are threaded into the template context (agitmap), so the AppKitdatabricks.yml.tmplcan render the block conditionally.Detection is conservative and falls back to
source_code_pathwhen there is nothing safe to point a deploy at: no repo, no origin remote, an unrecognized/self-hosted provider host, a detached HEAD, or a destination outside the repo. It therefore never emits an unusable git block.Why
Git-backed deployment (deploy from a repo/ref rather than uploading local files) is the recommended path for Databricks Apps — it is reproducible, reviewable, and rollback-able. Today a scaffolded app defaults to a local
source_code_pathand the git block is a commented placeholder the user must fill in by hand, so almost nobody adopts it. Auto-detecting the repo the user is already standing in makes git-backed the default with zero extra steps, while degrading cleanly to the old behavior everywhere else.Pairs with the AppKit template change in databricks/appkit#556 (the template renders the
git_repository/git_sourceblock from these fields). Backward compatible: an older CLI renders the git fields empty and the template keepssource_code_path.Tests
cmd/apps/gitsource_test.gocover provider inference, origin-URL normalization (https,.gitsuffix, scp-style ssh, credential stripping, unknown hosts), anddetectGitScaffoldSourceacross repo-root/in-place, subdirectory, not-yet-created subdir, no-repo, no-origin, unknown-provider, and detached-HEAD cases.This pull request and its description were written by Isaac.