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

feat: add an install script for easy install #95

Merged
merged 4 commits into from
Nov 29, 2023
Merged

Conversation

pottekkat
Copy link
Contributor

@pottekkat pottekkat commented Nov 16, 2023

Description

Adds an install script that users can just run, and it will install the appropriate version of the ADC binary for macOS and Linux.

TODO:

We need to serve this file similar to how we serve https://run.api7.ai/apisix/quickstart

I suggest this URL https://run.api7.ai/adc/install

Once we do that, users can install ADC by just running curl -sL https://run.api7.ai/adc/install | sh.

@juzhiyuan Can you help with this?

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible

Signed-off-by: Navendu Pottekkat <navendu@apache.org>
@juzhiyuan
Copy link
Contributor

Hi @pottekkat, you can check https://github.com/api7/docs/blob/main/.github/workflows/update-s3-and-cloudfront.yaml#L28 for deploying resources on that domain.

For this PR, please let Shirui Zhao to assign people to review. Thanks

@nfrankel
Copy link

Great intiative!

Copy link

@AlinsRan AlinsRan left a comment

Choose a reason for hiding this comment

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

Running it, installation failed:

image

README.md Outdated
```
go install github.com/api7/adc@latest

Choose a reason for hiding this comment

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

Why remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For users, it is better to directly download the binary instead of installing with go install. Now I think maybe we should keep both.

Choose a reason for hiding this comment

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

Not really. What's the benefit of keeping an installation approach coupled to a tech stack?

README.md Outdated
Comment on lines 27 to 28
wget https://github.com/api7/adc/releases/download/v0.5.0/adc_0.5.0_linux_amd64.tar.gz
tar -zxvf adc_0.5.0_linux_amd64.tar.gz

Choose a reason for hiding this comment

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

We can use environment variables to specify the version.

@pottekkat
Copy link
Contributor Author

pottekkat commented Nov 17, 2023

@AlinsRan Thanks for the review. Checking what the error is. We need to improve error handling.

@@ -0,0 +1,84 @@
#!/usr/bin/env bash

ARCH="$(uname -m)"

Choose a reason for hiding this comment

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

image

Choose a reason for hiding this comment

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

image

Signed-off-by: Navendu Pottekkat <navendu@apache.org>
Signed-off-by: Navendu Pottekkat <navendu@apache.org>
@pottekkat
Copy link
Contributor Author

@AlinsRan Is it working now? I tested in a Linux environment as well. If it is working, I will add more error handling to the script.

Screenshot 2023-11-20 at 14 51 06

Signed-off-by: Navendu Pottekkat <navendu@apache.org>
Copy link

@AlinsRan AlinsRan left a comment

Choose a reason for hiding this comment

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

A small suggestion:

image

@juzhiyuan juzhiyuan merged commit 13594d4 into main Nov 29, 2023
8 checks passed
@juzhiyuan juzhiyuan deleted the feat/install-script/1 branch November 29, 2023 08:58
bzp2010 added a commit to bzp2010/adc that referenced this pull request Jul 12, 2024
bzp2010 pushed a commit to bzp2010/adc that referenced this pull request Jul 12, 2024
bzp2010 added a commit to bzp2010/adc that referenced this pull request Jul 12, 2024
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.

5 participants