Staubgeborener / klipper-backup

Klipper backup script for manual or automated GitHub backups. Lightweight, pragmatic and comfortable.
https://klipperbackup.xyz
220 stars 46 forks source link

Install.sh script #73

Closed Tylerjet closed 3 months ago

Tylerjet commented 4 months ago

As a draft for now, can be tested with git pull origin pull/73/head

Tylerjet commented 4 months ago

There are some lines i will like update/move to functions for ease of modifying/editing in the future but it is in a state now where the main actions can be tested.

Staubgeborener commented 4 months ago

First of all: what the heck, that's a lot of code.

I tested this on my test machine which isn't using moonraker at all (right now). So while trying the installation i'll get this error:

● Adding klipper-backup to update manager. ● Restarting moonraker. Failed to restart moonraker.service: Unit moonraker.service not found.

... and the script simply aborts and doesn't try anything else. Maybe a red error message, doing the rest of the installation and warn the user to check moonraker and do an sudo systemctl restart moonraker.service by himself?

Btw: After running the script a second time after this error, i don't get the moonraker error message. The script simply skips this section and tries to update/install inotify-tools.

Is there also the chance to integrate a "step back" button? I tried the installation and copy/paste my .env stuff inside the asked question, but at one time my terminal thought the input is "empty" (like there is nothing in my clipboard) and the next question was at the same line like the old question. At this point i wasn't sure if i give the script the correct parameter or not. Same thing if i hit enter a little bit to fast like this:

Enter your github token: ***** Enter your github username: Enter your repository name:

With a "step back" command i could be sure about it.

Tylerjet commented 4 months ago

Hehe yeah it looks like a lot but probably 25% of it is just the text that gets pushed into service files lol.

Re: moonraker that should be simple enough, On the second run it would have looked for the moonraker.conf and checked if it already had the klipper backup entry added and if it does it skips that step so that is expected.

I'm not sure about a single step back but could likely do something like a key combination that restarts that configuration section, will see on that one.

Staubgeborener commented 4 months ago

Someone in Minimal3Ds video mentioned, that the timestamp format is inconsistent. Maybe we could also fix this with this PR, as we also create a cronjob with this install script which handles the timestamp format.

image

Tylerjet commented 4 months ago

hrmm which macro are they referring to i wonder as i though i had updated them all, but yeah I'll go through and double check they all match the new format of $(date +"%x - %X")

Tylerjet commented 4 months ago

ahh yeah i see backup on boot still had %%D instead of the new formatt will be in next commit, will need to update that in the docs too

Tylerjet commented 3 months ago

I really hate the look of whiptail compared to the rest of the script but hey it seems to work, might look at another way to trigger going back that i could go back to all text based.

Staubgeborener commented 3 months ago

Is this PR finished and ready for testing or do you want to change something?

Tylerjet commented 3 months ago

Let me do a full test tomorrow I would still like to find a better way of a go back feature as the current input style seems very off from the style of the rest of the script. But that's technically something I can improve in the future. But if you do want to test it on your end it can be pulled with git pull origin pull/73/head

Tylerjet commented 3 months ago

More changes will be coming as i think i found the direction and information i wanted to streamline the process. 🎉

Tylerjet commented 3 months ago

@Staubgeborener definitely test from that latest commit

Staubgeborener commented 3 months ago

Looks pretty good so far. Maybe add a check for install_backup_service(), install_cron() and install_filewatch_service() if these service are already installed? So only ask to install those services if they aren't already installed.

Tylerjet commented 3 months ago

Merging this into the newly created installer-dev branch instead of into main for now while its worked on but this way its not still sitting as a pending PR.