-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Add proper Adguard Home update #8785
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
Conversation
ct/adguard.sh
Outdated
| if check_for_gh_release "AdGuardHome" "AdguardTeam/AdGuardHome"; then | ||
| read -r -p "It is recommended to update AdGuard Home from the web interface. Would you like to continue with a manual update? <y/N> " prompt | ||
| if [[ "${prompt,,}" =~ ^(y|yes)$ ]]; then | ||
| # After stopping Adguard Home service, github.com might not resolve anymore -> first install into temp location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the dislike for comments, especially if they explain the "why" (which you cannot learn from the code). It will likely prevent a future maintainer running into the same issues. Nevertheless, if it's what you want, I will remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that was one of the things we adopted from tteck, maybe if you can shorten it a bit down I'm okay with it, what about other @community-scripts/contributor ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah that's why. He was a single maintainer so probably he remembered all the "why"s ;)
Indeed something for broader discussion, you want to avoid useless comment like
# removing /opt/app
rm -rf /opt/app
but some things you just can't learn from the code, and then they might be useful
|
Wait, is Adguard updated via webui? |
|
@tremor021 Typically, yes. It's a one-click thing, so really convenient. (and it avoids that you have to stop adguard to do a manual update, by which point you might have no DNS anymore) Explained in the PR: web interface update might fail, see the Adguard website linked in the PR |
|
hmm honestly that's a good point, I've been running adguard since over a year and regularly update without having ever received an error. If AdGuard recommends update via webUI, so I don't see a reason to add manual update? |
|
I also had it running (and updating) for over a year, but somehow it failed for me this morning and pointed me to the adguard documentation suggesting to do it manually: https://adguard-dns.io/kb/adguard-home/faq/#manual-update-unix:~:text=If%20the%20button%20isn%E2%80%99t%20displayed%20or%20an%20automatic%20update%20has%20failed%2C%20you%20can%20update%20manually. But indeed the web update is preferred, hence the note, and the prompt directing people to the web update first |
|
hmm I think then this was rather a result of a faulty update from adguard, maybe? For me it worked fine and I'm on |
|
Yes, the update from adguard failed (v0.107.67 -> v0.107.69), but no clue why. It just told me to do a manual update, so I thought I would make this process a bit easier. |
|
@burgerga Discussed this interally again, as the tools has it's own update mechanic, this superseed all needs for an update mechanic from our side which would only need additional maintenance. |
|
Yeah, no problem! |
✍️ Description
Currently Adguard is listed as updateable, but the only thing it does is
msg_error .... Update via the web interface might fail though (happened to me this morning), which is why Adguard has also provided manual instructions. This PR will implement the manual update. It also takes into account that github.com might not resolve anymore after stopping the Adguard service (again, happened to me, as Adguard is my only DNS server :P), so it first installs Adguard into a temporary location, and then stops the service and moves it to the correct location.🔗 Related PR / Issue
Link: #
✅ Prerequisites (X in brackets)
🛠️ Type of Change (X in brackets)
README,AppName.md,CONTRIBUTING.md, or other docs.