astronomer / astro-cli

CLI that makes it easy to create, test and deploy Airflow DAGs to Astronomer
https://www.astronomer.io
Other
341 stars 70 forks source link

Upgrade notice goes to stdout, which breaks completion #1583

Closed danielhoherd closed 2 months ago

danielhoherd commented 4 months ago

Describe the bug

The notice that there is a new version available goes to stdout, which breaks bash completion for the astro command.

$ astro completion bash 2> /dev/null | head

A newer version of Astro CLI is available: 1.24.1
Please update to the latest version using 'brew upgrade astro'

If you don't want to see this message again run 'astro config set -g upgrade_message false'

# bash completion V2 for astro                                -*- shell-script -*-

__astro_debug()
{

The fix is to make the following text go to stderr instead of stdout:

A newer version of Astro CLI is available: 1.24.1
Please update to the latest version using 'brew upgrade astro'

If you don't want to see this message again run 'astro config set -g upgrade_message

Alternatively we could prefix every line in the notice with a # which would cause those lines to be interpreted as shell code comments.

What CLI Version did you experience this bug?

1.23.0, 1.24.0. I cannot test on 1.24.1 because the bug requires a newer version to be available in order to trigger.

This CLI bug is related to which Astronomer Platform?

What Operating System is the above CLI installed on?

macOS 14

🪜 Steps To Reproduce

  1. Run a not-latest-version of astro-cli
  2. Run . <(astro completion bash) or whatever your shell is
  3. Observe a syntax error:
$ . <(astro completion bash)
-bash: A: command not found
-bash: Please: command not found
-bash: If: command not found
kushalmalani commented 4 months ago

You can run the following command to stop upgrade new version messages

astro config set -g upgrade_message false

danielhoherd commented 4 months ago

@kushalmalani Right, but that disables a feature in order to work around the root cause of the problem. We should allow users to be alerted about a new version without breaking their completion.

sunkickr commented 3 months ago

@kushalmalani I agree we should disable this message when running this command

danielhoherd commented 2 months ago

This hit me again today.

$ astro version

A newer version of Astro CLI is available: 1.26.0
Please update to the latest version using 'brew upgrade astro'

If you don't want to see this message again run 'astro config set -g upgrade_message false'

Astro CLI Version: 1.25.0
$ astro completion bash | head

A newer version of Astro CLI is available: 1.26.0
Please update to the latest version using 'brew upgrade astro'

If you don't want to see this message again run 'astro config set -g upgrade_message false'

# bash completion V2 for astro                                -*- shell-script -*-

__astro_debug()
{

I automatically regenerate completion files periodically so I can make sure I always have the latest ones. This automatically showed up in my terminal because of the release of the new version.

$ head ~/.bash_completion.d/astro

A newer version of Astro CLI is available: 1.26.0
Please update to the latest version using 'brew upgrade astro'

If you don't want to see this message again run 'astro config set -g upgrade_message false'

# bash completion V2 for astro                                -*- shell-script -*-

__astro_debug()
{

I think wrapping this text in # characters is the easiest way to solve this problem.

kushalmalani commented 2 months ago

@danielhoherd Fix is in review. Will most likely be merged next week and released soon

danielhoherd commented 2 months ago

@kushalmalani thanks! I was just opening a PR with a fix.

kushalmalani commented 2 months ago

@danielhoherd I merged the PR with this fix. Fix will be released next week as part of 1.27.0