if statement for version check fix #1369
Conversation
if statement for version check fix
afastronomer
requested review from
neel-astro,
kushalmalani and
sunkickr
as code owners
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #1369 +/- ##
=======================================
Coverage 86.89% 86.89%
=======================================
Files 114 114
Lines 13912 13912
=======================================
Hits 12089 12089
Misses 1087 1087
Partials 736 736 ☔ View full report in Codecov by Sentry. |
godownloader.sh
Outdated
| version=$(curl https://api.github.com/repos/${owner_repo}/releases/latest -s | tr -s '\n' ' ' | sed 's/.*"tag_name": "//' | sed 's/".*//') | ||
| elif [[ "$version" =~ ^v[0-9]+\.[0-9]+$ ]]; then | ||
| elif [ $version = ^v[0-9]+\.[0-9]+$ ]; then |
There was a problem hiding this comment.
Do we need to keep version as string? or does this work for you regardless
There was a problem hiding this comment.
I think we might need to continue to use double brackets, in order to support regex in the elif clause. Any reason to change this as well?
It is supposed to support a case when users try to send in a request to install a version without mentioning the patch number, then that logic would pick up the latest patch for the mentioned version.
The ideal output would be that v1.18.2 is installed when we pass v1.18, but with the current change, it is not able to find any version.
neel@Neels-MacBook-Pro astro-cli % bash godownloader.sh -- v1.18
astronomer/astro-cli info checking GitHub for tag 'v1.18'
astronomer/astro-cli crit unable to find 'v1.18' - use 'latest' or see https://github.com/astronomer/astro-cli/releases for details
neel@Neels-MacBook-Pro astro-cli %
Ideal output:
neel@Neels-MacBook-Pro astro-cli % bash godownloader.sh -- v1.18
astronomer/astro-cli info checking GitHub for tag 'v1.18'
astronomer/astro-cli info found version: 1.18.2 for v1.18.2/darwin/arm64
astronomer/astro-cli info installed ./bin/astro
neel@Neels-MacBook-Pro astro-cli %
godownloader.sh
Outdated
| version=$(curl https://api.github.com/repos/${owner_repo}/releases/latest -s | tr -s '\n' ' ' | sed 's/.*"tag_name": "//' | sed 's/".*//') | ||
| elif [[ "$version" =~ ^v[0-9]+\.[0-9]+$ ]]; then | ||
| elif [ $version = ^v[0-9]+\.[0-9]+$ ]; then |
There was a problem hiding this comment.
I think we might need to continue to use double brackets, in order to support regex in the elif clause. Any reason to change this as well?
It is supposed to support a case when users try to send in a request to install a version without mentioning the patch number, then that logic would pick up the latest patch for the mentioned version.
The ideal output would be that v1.18.2 is installed when we pass v1.18, but with the current change, it is not able to find any version.
neel@Neels-MacBook-Pro astro-cli % bash godownloader.sh -- v1.18
astronomer/astro-cli info checking GitHub for tag 'v1.18'
astronomer/astro-cli crit unable to find 'v1.18' - use 'latest' or see https://github.com/astronomer/astro-cli/releases for details
neel@Neels-MacBook-Pro astro-cli %
Ideal output:
neel@Neels-MacBook-Pro astro-cli % bash godownloader.sh -- v1.18
astronomer/astro-cli info checking GitHub for tag 'v1.18'
astronomer/astro-cli info found version: 1.18.2 for v1.18.2/darwin/arm64
astronomer/astro-cli info installed ./bin/astro
neel@Neels-MacBook-Pro astro-cli %
if statement for version check fix
Description
In some shells
ifstatement doesn't work correct.🎟 Issue(s)
Related #XXX
🧪 Functional Testing
Script was executed w/ and w/o specified value for version.
📋 Checklist
make testbefore taking out of draftmake lintbefore taking out of draft