Skip to content

Revert spinner to fix telepresence clientside apply step#5

Merged
scheibinger merged 2 commits intomainfrom
cop-1414/revert-spinner
Jul 21, 2025
Merged

Revert spinner to fix telepresence clientside apply step#5
scheibinger merged 2 commits intomainfrom
cop-1414/revert-spinner

Conversation

@scheibinger
Copy link
Copy Markdown
Contributor

@scheibinger scheibinger commented Jul 21, 2025

Adding combined output in the spinner, broke the telepresence client side apply which was prompting for the user admin password.

It was showing the error below:

⣻ Executing configuration...Need root privileges to run: /opt/homebrew/bin/telepresence daemon-foreground /Users/radek/Library/Logs/telepresence '/Users/radek/Library/Application Support/telepresence'

Copilot AI review requested due to automatic review settings July 21, 2025 11:27
@scheibinger scheibinger requested a review from a team as a code owner July 21, 2025 11:27
@github-actions
Copy link
Copy Markdown
Contributor

👋 scheibinger, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reverts changes to the spinner functionality in the client-side apply component to fix a bug where telepresence client-side apply was unable to prompt for user admin password. The removal of combined output handling in the spinner restored the ability for interactive prompts to work properly.

  • Removes spinner progress indication from command execution
  • Restores proper stdout/stderr streaming for interactive commands
  • Eliminates spinner configuration and related dependencies

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/adapter/clientsideapply/cmd.go Removes spinner usage and restores proper I/O handling for interactive commands
internal/adapter/clientsideapply/client_side_apply.go Removes spinner configuration and yacspin dependency

import (
"context"
"fmt"
"io"
Copy link

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The io import is added but consider if all spinner-related cleanup was properly handled. Verify that no orphaned references to spinner functionality remain in other parts of the codebase.

Copilot uses AI. Check for mistakes.
@scheibinger scheibinger merged commit 1f59c7b into main Jul 21, 2025
7 checks passed
@scheibinger scheibinger deleted the cop-1414/revert-spinner branch July 21, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants