Skip to content

Add elapsed time display to spinners#4697

Open
anton-107 wants to merge 1 commit intossh-connect-ux-spinnersfrom
ssh-connect-elapsed-time
Open

Add elapsed time display to spinners#4697
anton-107 wants to merge 1 commit intossh-connect-ux-spinnersfrom
ssh-connect-elapsed-time

Conversation

@anton-107
Copy link
Contributor

Summary

  • Add TrackElapsedTime() method to the spinner that shows a running MM:SS stopwatch next to the spinner message
  • The elapsed time updates on every spinner tick (200ms) with no extra goroutines
  • Enable it for all SSH connect spinners (binary upload, cluster check, job startup, metadata polling)

Example output: ⣾ Starting SSH server... (task: PENDING) 00:15

Stacked on #4694

Test plan

  • Run databricks ssh connect --cluster <id> and verify elapsed time appears and ticks next to each spinner
  • Verify other spinners in the codebase (without TrackElapsedTime()) are unaffected

🤖 Generated with Claude Code

Add TrackElapsedTime() method to the spinner that shows a running
MM:SS stopwatch next to the spinner message. The time updates on
every spinner tick (200ms). Enable it for all SSH connect spinners
so users can see how long each step has been running.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@anton-107 anton-107 force-pushed the ssh-connect-elapsed-time branch from 284aa8e to c8eceee Compare March 10, 2026 16:35
@anton-107 anton-107 temporarily deployed to test-trigger-is March 10, 2026 16:35 — with GitHub Actions Inactive
@eng-dev-ecosystem-bot
Copy link
Collaborator

eng-dev-ecosystem-bot commented Mar 10, 2026

Commit: c8eceee

Run: 22913294018

Env 🔄​flaky 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
💚​ aws linux 8 7 268 781 6:50
💚​ aws windows 8 7 270 779 5:50
🔄​ aws-ucws linux 2 7 7 364 696 16:56
🔄​ aws-ucws windows 2 7 7 366 694 13:26
💚​ azure linux 2 9 271 779 6:15
💚​ azure windows 2 9 273 777 5:45
🔄​ azure-ucws linux 2 1 9 369 692 10:31
🔄​ azure-ucws windows 2 1 9 371 690 10:28
💚​ gcp linux 2 9 267 782 5:33
💚​ gcp windows 2 9 269 780 5:06
16 interesting tests: 7 SKIP, 6 RECOVERED, 3 flaky
Test Name aws linux aws windows aws-ucws linux aws-ucws windows azure linux azure windows azure-ucws linux azure-ucws windows gcp linux gcp windows
🔄​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 🔄​f 🔄​f 💚​R 💚​R
🙈​ TestAccept/bundle/resources/permissions 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions 💚​R 💚​R 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct 💚​R 💚​R 💚​R 💚​R
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 💚​R 💚​R 💚​R 💚​R
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions 💚​R 💚​R 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct 💚​R 💚​R 💚​R 💚​R
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/resources/postgres_branches/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/update_protected 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/without_branch_id 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_endpoints/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/synced_database_tables/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🔄​ TestAccept/ssh/connect-serverless-gpu 🙈​s 🙈​s 🔄​f 🔄​f 🙈​s 🙈​s 🔄​f 🔄​f 🙈​s 🙈​s
🔄​ TestAccept/ssh/connection 💚​R 💚​R 🔄​f 🔄​f 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
Top 22 slowest tests (at least 2 minutes):
duration env testname
5:01 aws-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
4:43 aws-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
4:19 aws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
4:00 aws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:51 aws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:46 aws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:46 gcp windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:34 aws-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:31 aws-ucws windows TestAccept/bundle/resources/permissions/jobs/delete_one/cloud/DATABRICKS_BUNDLE_ENGINE=terraform
3:29 aws-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:24 azure-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:15 gcp linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:08 gcp windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:07 gcp linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:01 azure-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:50 azure windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:45 azure linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:25 azure-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:23 azure-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:14 azure linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:04 azure windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:00 aws-ucws linux TestAccept/bundle/deployment/bind/job/generate-and-bind/DATABRICKS_BUNDLE_ENGINE=terraform

return m.spinner.View()
if !m.startTime.IsZero() {
elapsed := time.Since(m.startTime)
result += fmt.Sprintf(" %02d:%02d", int(elapsed.Minutes()), int(elapsed.Seconds())%60)
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if there is no suffix yet?

spinner bubblespinner.Model
suffix string
quitting bool
startTime time.Time // non-zero when elapsed time display is enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend composing this as a "time spinner" that wraps this spinner and adds the time when View() is called. Then construct it with NewTimeSpinner.

Copy link
Contributor

@pietern pietern Mar 12, 2026

Choose a reason for hiding this comment

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

This suggestion is prob overkill.

Re: the feature, I think doing prefixing instead of suffixing makes more sense because it won't jump around as updates happen. Second, the time can be initialized at construction time instead of a separate method, and then the time prefix as an option to the spinner at construction time.

simonfaltum
simonfaltum previously approved these changes Mar 12, 2026
Copy link
Member

@simonfaltum simonfaltum left a comment

Choose a reason for hiding this comment

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

[Agent Swarm Review] Verdict: Approved

  • 0 Critical
  • 0 Major
  • 0 Gap
  • 1 Nit
  • 0 Suggestion

Clean addition of elapsed time tracking to spinners. The TrackElapsedTime() method is well-integrated into the Bubble Tea model. Minor note: the MM:SS format doesn't handle hours, but SSH connect operations shouldn't take that long.

@simonfaltum simonfaltum dismissed their stale review March 12, 2026 09:42

Too aggressive isaac

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants