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] Install latest published release from GitHub #407 #412

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

youhaveme9
Copy link
Contributor

@youhaveme9 youhaveme9 commented Dec 14, 2024

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #407.

Description of Changes

Updated auto-install script to get the latest release from GitHub by default instead of installing the release with "latest" tag.

Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

Good work @youhaveme9!

I have tested this and it works as expected. I have left some minor suggestions below. Once, those changes are made we can go ahead and merge this PR.

@@ -54,10 +54,18 @@ error_msg_with_continue() {

apt_dependenices_setup() {
start_step "Setting up dependencies..."
apt --yes install python3 python3-pip git python3-dev gawk libffi-dev libssl-dev gcc make &>>$LOG_FILE
apt --yes install python3 python3-pip git python3-dev gawk libffi-dev libssl-dev gcc make curl &>>$LOG_FILE
Copy link
Member

Choose a reason for hiding this comment

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

Let's also include jq, so we don't into similar errors like curl.

echo -ne ${GRN}"OpenWISP Version (leave blank for latest): "${NON}
read openwisp_version
if [[ -z "$openwisp_version" ]]; then
openwisp_version=$(curl -L --silent https://api.github.com/repos/openwisp/docker-openwisp/releases/latest | jq .tag_name | tr -d '"')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
openwisp_version=$(curl -L --silent https://api.github.com/repos/openwisp/docker-openwisp/releases/latest | jq .tag_name | tr -d '"')
openwisp_version=$(curl -L --silent https://api.github.com/repos/openwisp/docker-openwisp/releases/latest | jq -r .tag_name)

@youhaveme9
Copy link
Contributor Author

Hey @pandafy
Updated the script as required

Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

LGTM!

@pandafy pandafy requested a review from nemesifier December 16, 2024 15:25
@pandafy pandafy changed the title [fix] Fixed bug in auto-install #407 [fix] Install the latest published release from GitHub #407 Dec 16, 2024
@pandafy pandafy changed the title [fix] Install the latest published release from GitHub #407 [fix] Install latest published release from GitHub #407 Dec 16, 2024
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

THanks @youhaveme9 @pandafy, I merged the latest master and started the build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[bug] Auto install script point to tag named "latest"
3 participants