Skip to content

Comments

Fix runtime spinner issues#1779

Merged
pritt20 merged 9 commits intomainfrom
fix_spinner
Jan 21, 2025
Merged

Fix runtime spinner issues#1779
pritt20 merged 9 commits intomainfrom
fix_spinner

Conversation

@pritt20
Copy link
Contributor

@pritt20 pritt20 commented Jan 6, 2025 •
edited
Loading

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
neel-astro
neel-astro approved these changes Jan 17, 2025
View reviewed changes
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

@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
@pritt20 @schnie @neel-astro
Fix runtime spinner issues (#1779)
bea0422 
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

Reviewers

@neel-astro neel-astro neel-astro approved these changes

@jeremybeard jeremybeard Awaiting requested review from jeremybeard

@kushalmalani kushalmalani Awaiting requested review from kushalmalani

@schnie schnie Awaiting requested review from schnie

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pritt20 @schnie @neel-astro