Skip to content

experimental/air: open MLflow URLs via libs/browser instead of pkg/browser - #6752

Open
pietern wants to merge 1 commit into
mainfrom
air-browser-fix
Open

pietern wants to merge 1 commit into
mainfrom
air-browser-fix

Conversation

@pietern

@pietern pietern commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Changes

air list's TUI opened the selected run's MLflow URL with github.com/pkg/browser directly. Switch it to libs/browser.Open(ctx, url), threading the fetcher's context through to the call site.

Why

pkg/browser ignores the BROWSER env var, so BROWSER=none and custom browser commands break. libs/browser is the repo's required wrapper for opening URLs.

Tests

go build, go vet, go test ./experimental/air/cmd -run TestListModel (11/11), and the air acceptance suite pass.

This pull request and its description were written by Isaac.

… env var support

Replace direct usage of github.com/pkg/browser with github.com/databricks/cli/libs/browser
to properly respect the BROWSER environment variable. This allows BROWSER=none and custom
browser commands to work correctly.

- Convert openURL from a standalone function to a method on listModel to access the context
- Use browser.Open(ctx, url) with the proper context from the fetcher
- Extract context before creating the closure to avoid use-after-free (closure captures value, not receiver)
- Check for nil fetcher at call site, consistent with other interactive key handlers
- Preserve best-effort behavior by ignoring errors
Co-authored-by: Isaac <no-reply@databricks.com>
@github-actions

Copy link
Copy Markdown
Contributor

Waiting for approval

Based on git history, these people are best suited to review:

  • @riddhibhagwat-db -- recent work in experimental/air/cmd/

Eligible reviewers: @apeforest, @ben-hansen-db, @bfontain, @caroline-db, @maggiewang-db, @panchalhp-db, @pardis-beikzadeh-db, @vinchenzo-db

Suggestions based on git history. See OWNERS for ownership rules.

@pietern
pietern enabled auto-merge September 18, 2026 11:31
@pietern pietern added the AIR Databricks AI Runtime CLI label Sep 18, 2026
@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 5bda4e5

Run: 35340060457

Env ✅​pass 🙈​skip Time
✅​ aws linux 276 15 5:49
✅​ aws windows 278 13 4:16
✅​ azure linux 275 15 5:58
✅​ azure windows 277 13 4:06
✅​ gcp linux 276 15 7:24
✅​ gcp windows 278 13 4:04
Top 3 slowest tests (at least 2 minutes):
duration env testname
3:54 aws windows TestAccept
3:51 azure windows TestAccept
3:41 gcp windows TestAccept

ctx := m.fetcher.ctx
return func() tea.Msg {
_ = browser.OpenURL(url)
_ = browser.Open(ctx, url)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A data race occurs and is printed by pressing Enter twice quickly. Bubble Tea runs both browser commands concurrently, while openDefault mutates global browserpkg.Stderr. Please guard the save/set/open/restore sequence

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AIR Databricks AI Runtime CLI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants