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

New Script: Hoarder LXC #567

Merged
merged 29 commits into from
Dec 2, 2024
Merged

New Script: Hoarder LXC #567

merged 29 commits into from
Dec 2, 2024

Conversation

vhsdream
Copy link
Contributor

Note

We are meticulous when it comes to merging code into the main branch, so please understand that we may reject pull requests that do not meet the project's standards. It's never personal. Also, game-related scripts have a lower chance of being merged.

Description

With help from @MickLesk and MohamedBasem I have created an LXC install script for Hoarder, the bookmark everything app.

Type of change

Please check the relevant option(s):

  • Bug fix (non-breaking change that resolves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (a fix or feature that would cause existing functionality to change unexpectedly)
  • New script (a fully functional and thoroughly tested script or set of scripts.)

Prerequisites

The following efforts must be made for the PR to be considered. Please check when completed:

  • Self-review performed (I have reviewed my code, ensuring it follows established patterns and conventions)
  • Testing performed (I have tested my changes, ensuring everything works as expected)
  • Documentation updated (I have updated any relevant documentation)

Additional Information (optional)

Provide any additional context or screenshots about the feature or fix here.

For some reason the bazaar scripts were executable so I changed their modes, that's why you'll see them in this PR. I'll need to learn to not get distracted by little things like that (or make a separate PR) when I try to contribute.

Related Pull Requests / Discussions

If there are other pull requests or discussions related to this change, please link them here:

@vhsdream vhsdream requested a review from a team as a code owner November 28, 2024 12:58
@github-actions github-actions bot added new script A change that adds a new script update script A change that updates a script labels Nov 28, 2024
@MohamedBassem
Copy link
Contributor

Thanks a lot for all your effort in adding hoarder to proxmox scripts. I'll hopefully be able to review the hoarder side of this PR later tonight!

Copy link
Member

@MickLesk MickLesk left a comment

Choose a reason for hiding this comment

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

LGTM, 1 comment. @havardthom need to check too

install/hoarder-install.sh Outdated Show resolved Hide resolved
Also made similar changes in some other areas.
ct/hoarder.sh Outdated Show resolved Hide resolved
ct/hoarder.sh Outdated Show resolved Hide resolved
ct/hoarder.sh Show resolved Hide resolved
ct/hoarder.sh Outdated Show resolved Hide resolved
ct/hoarder.sh Outdated Show resolved Hide resolved
install/hoarder-install.sh Outdated Show resolved Hide resolved
install/hoarder-install.sh Outdated Show resolved Hide resolved
install/hoarder-install.sh Outdated Show resolved Hide resolved
install/hoarder-install.sh Outdated Show resolved Hide resolved
install/hoarder-install.sh Show resolved Hide resolved
@MickLesk MickLesk removed the update script A change that updates a script label Nov 28, 2024
@github-actions github-actions bot added breaking change A change that is not backward compatible update script A change that updates a script delete script A change that deletes a script labels Nov 28, 2024
MickLesk and others added 5 commits November 28, 2024 15:55
Co-authored-by: Håvard Gjøby Thom <[email protected]>
Co-authored-by: Håvard Gjøby Thom <[email protected]>
Co-authored-by: Håvard Gjøby Thom <[email protected]>
Co-authored-by: Håvard Gjøby Thom <[email protected]>
ct/hoarder.sh Outdated Show resolved Hide resolved
ct/hoarder.sh Show resolved Hide resolved
ct/hoarder.sh Outdated Show resolved Hide resolved
ct/hoarder.sh Outdated Show resolved Hide resolved
@MickLesk MickLesk removed update script A change that updates a script breaking change A change that is not backward compatible labels Nov 28, 2024
@MickLesk MickLesk removed the delete script A change that deletes a script label Nov 28, 2024
@github-actions github-actions bot added the website A change to the website label Nov 28, 2024
@MickLesk
Copy link
Member

Added JSON for website.

@MickLesk
Copy link
Member

Dont merge the Main Branch ^^ you can switch in GitHub to your pushed Branch and Change your File.

Now I have to rewrite your branch so that the changes are not duplicated.

@vhsdream
Copy link
Contributor Author

vhsdream commented Nov 29, 2024 via email

@MickLesk
Copy link
Member

No. I pick the Files later.

Next time Switch in GitHub to your Branch and edit the file there. 😄

MickLesk
MickLesk previously approved these changes Nov 29, 2024
install/hoarder-install.sh Outdated Show resolved Hide resolved
install/hoarder-install.sh Outdated Show resolved Hide resolved
$STD pnpm install --frozen-lockfile

HOARDER_SECRET=$(openssl rand -base64 36 | cut -c1-24)
cat <<EOF >/opt/hoarder/.env
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be in /etc/hoarder/config.env instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I originally had the env file in /etc/hoarder.env as per your suggestion, it was implied to me that it is not standard to do that for scripts in this repo. I'll leave it up to the maintainers to decide how to proceed with this sort of thing.

install/hoarder-install.sh Outdated Show resolved Hide resolved
msg_ok "Stopped Services"
msg_info "Updating ${APP} to v${RELEASE}"
cd /opt
mv /opt/hoarder/.env /opt/.env
Copy link
Contributor

Choose a reason for hiding this comment

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

This is another reason why the env files should probably be in /etc/hoarder instead.

ct/hoarder.sh Show resolved Hide resolved
PREV_RELEASE=$(cat /opt/${APP}_version.txt)
if [[ ! -f /opt/${APP}_version.txt ]] || [[ "${RELEASE}" != "${PREV_RELEASE}" ]]; then
msg_info "Stopping Services"
systemctl stop hoarder-web hoarder-workers hoarder-browser
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, you don't need to stop the browser for a hoarder update

Copy link
Contributor

@MohamedBassem MohamedBassem left a comment

Choose a reason for hiding this comment

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

Looks good to me from Hoarder's side of things! People will typically want to configure the AI tagging, so suggested adding some place holders in the env file.

install/hoarder-install.sh Show resolved Hide resolved
@MohamedBassem
Copy link
Contributor

Once it lands, I'm planning to advertise it in the wiki as one of the supported installation mechanisms. Thanks a lot for working on this!

@MickLesk MickLesk merged commit 1ca9978 into community-scripts:main Dec 2, 2024
1 check passed
@vhsdream vhsdream deleted the hoarder branch December 2, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new script A change that adds a new script website A change to the website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants