Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix runtime spinner issues #1779

Merged
merged 9 commits into from
Jan 21, 2025
Merged

Fix runtime spinner issues #1779

merged 9 commits into from
Jan 21, 2025

Conversation

pritt20
Copy link
Contributor

@pritt20 pritt20 commented Jan 6, 2025

Description

We observed couple of issues related to progress spinner in Astro CLI during setting up local project:

  1. During local Astro project startup we do not see any meaningful information or progress to keep users updated.
  2. Runtime init messages is too long, causing the spinner to print on multiple lines repeatedly. This is usually caused due to small terminal window size.

Screenshot:

image

The changes in this PR address above issues and provides user meaningful information as we set-up local astro-project.

🎟 Issue(s)

Related #XXX

🧪 Functional Testing

Tested the changes locally, please find below screen recording for ref:

Screen.Recording.2025-01-17.at.1.58.22.AM.mov

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

@schnie
Copy link
Member

schnie commented Jan 6, 2025

@pritt20 I still see the issue if I shrink my terminal down smaller than the first line. I wonder how hard it would be to somehow hook into the terminal size and wrap the text to multiple lines accordingly. Or maybe cut off the line and end with ... or something.

@pritt20
Copy link
Contributor Author

pritt20 commented Jan 7, 2025

@pritt20 I still see the issue if I shrink my terminal down smaller than the first line. I wonder how hard it would be to somehow hook into the terminal size and wrap the text to multiple lines accordingly. Or maybe cut off the line and end with ... or something.

Thanks @schnie ! Yeah, I am able to reproduce it as well. I'll try other options to see if we can include terminal size dynamically.

@pritt20 pritt20 changed the title Fix text wrap issue with runtime spinner Fix runtime spinner issues Jan 15, 2025
@pritt20 pritt20 marked this pull request as ready for review January 17, 2025 10:09
Copy link
Contributor

@neel-astro neel-astro left a comment

Choose a reason for hiding this comment

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

overall LGTM, left some minor nits, but approving ahead of time

airflow/runtimes/command.go Outdated Show resolved Hide resolved
airflow/runtimes/podman_runtime.go Show resolved Hide resolved
@pritt20 pritt20 requested a review from schnie January 17, 2025 13:20
@pritt20 pritt20 merged commit bb318a6 into main Jan 21, 2025
5 checks passed
@pritt20 pritt20 deleted the fix_spinner branch January 21, 2025 16:13
neel-astro pushed a commit that referenced this pull request Jan 21, 2025
Co-authored-by: Pritesh Arora <[email protected]>
Co-authored-by: Greg Neiheisel <[email protected]>
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