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

Rebased updating OctoFarm + Spectacular log dump #421

Merged
merged 25 commits into from
Apr 18, 2021

Conversation

davidzwa
Copy link
Contributor

@davidzwa davidzwa commented Apr 8, 2021

OctoFarm internal update command.

  • Setup buttons on client for Update / Force Check
  • Create route for OctoFarm client to call
  • Create initial serverCommands function for updating OctoFarm. It needs to:
    • Check for the git folder as this will only work with git installs currently.
    • Run the git pull command
    • Check for "Already up-to-date status from git pull
    • Check for local changes to git folder and fail on update
  • Hook in a function for the force check for an update with an API endpoint so client may call at their whim.
  • Make the AmIAlive check deactivate or activate the client "Update" button when update available / not.
  • Add UI information from the update check call inc. node information and the rest from latest commit.
  • Add ApexCharts again as it was emptied
  • Check loggin only when necessary!
  • Split out file for Scripts/ServerCommnds classes
  • Put try and catch at top level, throw exceptions otherwise.
  • Refactor ejs templates to use .? operaters
  • system.ejs latest release url
  • cleanup system.ejs
  • Add airgapped boolean
  • Add log dump download button
  • Add log dump generate functions / utils
  • Collect log files
  • Generate pretty text output of system information
  • Double check zip correctly generates
  • Disable restart button when pm2 is not available
  • Add isPm2 check into restart command
  • Change force check button to just "Check for Update"
  • Change output to green for "Check for Update" and change message to a "Already up to date"
  • Add in the usage of "git status" to update command
  • Clean up text file output
  • Make sure update notification only appears to Admin user groups

@davidzwa davidzwa added this to Awaiting Review in 1.1.13 - Pushup Party Apr 8, 2021
@davidzwa davidzwa force-pushed the feature/update-octofarm branch 2 times, most recently from 83ca59b to 06c1de6 Compare April 9, 2021 14:39
@NotExpectedYet
Copy link
Member

@davidzwa Pushed what's done, will smash some more in the morning and hopefully get it finished.

@davidzwa
Copy link
Contributor Author

davidzwa commented Apr 10, 2021

I love how detailed your PR is so far, this will hit dev soon like this! Good code so far, really good work there as well. 🚀

@NotExpectedYet
Copy link
Member

I love how detailed your PR is so far, this will hit dev soon like this! Good code so far, really good work there as well. rocket

Thanks man! Good to hear. I'll clean up the history one the same like this as well.

@davidzwa davidzwa changed the title Rebased and squashed changes on updating OctoFarm Rebased updating OctoFarm Apr 11, 2021
@NotExpectedYet
Copy link
Member

NotExpectedYet commented Apr 11, 2021

@davidzwa All done, just the tests and function comments to go through and update with a cup of tea.

Need food :P

Attached example files that are generated and outputted.

octofarm_dump.zip

OctoFarm System Information

OctoFarm Version

1.1.13-prerelease

Are we connected to the internet?

✓ ✓ ✓ We are! ✓ ✓ ✓

Are we running with node?

✓ ✓ ✓ We are! ✓ ✓ ✓

Are we running under pm2?

✘ ✘ ✘ We are not! ✘ ✘ ✘

Are we running under nodemon?

✓ ✓ ✓ We are! ✓ ✓ ✓

Are we in a docker container?

✘ ✘ ✘ We are not! ✘ ✘ ✘

Platform

linux

Processor Arch

x64

System Uptime

4h, 54m, 10s

OctoFarm Uptime

3m, 14s

OctoPrint Versions

1.5.3

@NotExpectedYet NotExpectedYet changed the title Rebased updating OctoFarm Rebased updating OctoFarm + Spectacular log dump Apr 11, 2021
@davidzwa
Copy link
Contributor Author

davidzwa commented Apr 11, 2021

@NotExpectedYet I cannot read the info above effectively. Please consider a compact and truly boring output format to save our eyes. Also, we'd be able to extend it later with much more detailed reporting (compactness > cool and artsy-fartsy style)

-- OctoFarm System Information -- 
OctoFarm Version:  
  1.1.13-prerelease 
Internet connection:  
  ✓
Running with NodeJS:  
  ✓

-- OctoFarm Service -- 
Either running under pm2:  
  ✘
Or running under nodemon:  
  ✓
Or running within a docker container  
  ✘

-- System --
Platform:  
  linux
Processor Arch:  
  x64
System Uptime:  
  4h, 54m, 10s 
OctoFarm Uptime:  
  3m, 14s
OctoPrint Versions:  
  1.5.3

Some points of attention:

  • Restart button is available when we know we are under nodemon/docker and it will thus fail.
  • pm2 command is fired off when under nodemon (or docker), and I expect docker as well. I have pm2 on my system, and this is crashing when I run under nodemon (and I guess docker). I'd prefer if pm2 is not fired off at all, as we already know it with isPm2(...) and the alternative checks.
  • error output in logs, when not under pm2, results in empty line as shown below. Also stdout is used to check for error. stdout gives me '\n' which isNaN(parseInt(...)) doesnt make a lot of sense (although it might work). I'd just check for stdout === 0, if that makes sense. Not sure what stdout is supposed to give, but I thought it was text like what pm2 pid OctoFarm results in (which is not a code right? Correct me if Im wrong.)
    image
    (Error output on console)
  • force check github is not really forced at all: the button sends a confusing message to the user: like they are doing something which requires care. I'd change it to Check for update. The user doesnt need to know where it came from (releases or repo). We will change this later without the user knowing, They need not know where it came from, unless you have something else in mind here.
  • Force check results in yellow Noty, like a warning. I'd prefer green and the message body being something like

"OctoFarm is already up to date, you're all set!"

  • we can check git changes with git status before git pull, we can advise the user before they are left alone to their own resolve. Please run "git reset --hard"! Or click here to save your config db js to .env! Like parse the git status output and provide tips to the user to check what changes to cancel out. (I can help out there on this PR, or we'd need to agree to do it directly in a follow-up PR).

This PR is hot!

@NotExpectedYet
Copy link
Member

On it this morning

@NotExpectedYet
Copy link
Member

@NotExpectedYet I cannot read the info above effectively. Please consider a compact and truly boring output format to save our eyes. Also, we'd be able to extend it later with much more detailed reporting (compactness > cool and artsy-fartsy style)

-- OctoFarm System Information -- 
OctoFarm Version:  
  1.1.13-prerelease 
Internet connection:  
  ✓
Running with NodeJS:  
  ✓

-- OctoFarm Service -- 
Either running under pm2:  
  ✘
Or running under nodemon:  
  ✓
Or running within a docker container  
  ✘

-- System --
Platform:  
  linux
Processor Arch:  
  x64
System Uptime:  
  4h, 54m, 10s 
OctoFarm Uptime:  
  3m, 14s
OctoPrint Versions:  
  1.5.3

Some points of attention:

* Restart button is available when we know we are under nodemon/docker and it will thus fail.

* pm2 command is fired off when under nodemon (or docker), and I expect docker as well. I have pm2 on my system, and this is crashing when I run under nodemon (and I guess docker). I'd prefer if pm2 is not fired off at all, as we already know it with `isPm2(...)` and the alternative checks.

* error output in logs, when not under pm2, results in empty line as shown below. Also stdout is used to check for error. `stdout` gives me '\n' which `isNaN(parseInt(...))` doesnt make a lot of sense (although it might work). I'd just check for `stdout === 0`, if that makes sense. Not sure what stdout is supposed to give, but I thought it was text like what `pm2 pid OctoFarm` results in (which is not a code right? Correct me if Im wrong.)
  ![image](https://user-images.githubusercontent.com/6005355/114319184-9ebe8480-9b10-11eb-8cff-b7afe4921410.png)
  (Error output on console)

* force check github is not really forced at all: the button sends a confusing message to the user: like they are doing something which requires care. I'd change it to `Check for update`. The user doesnt need to know where it came from (releases or repo). We will change this later without the user knowing, They need not know where it came from, unless you have something else in mind here.

* Force check results in yellow Noty, like a warning. I'd prefer green and the message body being something like

"OctoFarm is already up to date, you're all set!"

* we can check git changes with `git status` before `git pull`, we can advise the user before they are left alone to their own resolve. Please run "git reset --hard"! Or click here to save your config db js to `.env`! Like parse the `git status` output and provide tips to the user to check what changes to cancel out. (I can help out there on this PR, or we'd need to agree to do it directly in a follow-up PR).

This PR is hot!

Regarding the PID check, that was there before you'd made the pm2 function. All it's doing is checking for the output of a of a PID number. No number and it returns the error, but yeah your right in that it could be cleaner. I'll move it over to the pm2 check command then there's no need for it.

@NotExpectedYet
Copy link
Member

--- OctoFarm System Information ---

OctoFarm Version
 1.1.13-prerelease 
Are we connected to the internet?
  ✓  
Are we running with node?
  ✓  
Are we running under pm2?
  ✘ 
Are we running under nodemon?
  ✓  
Are we in a docker container?
  ✘ 
Platform
 linux 
Processor Arch
 x64 
System Uptime
 2d, 2h, 42m, 18s 
OctoFarm Uptime
  26s 
OctoPrint Versions
  1.5.3  

@davidzwa
Copy link
Contributor Author

davidzwa commented Apr 17, 2021

Why are the checks not firing I wonder, are the workflows changed?

@NotExpectedYet
Copy link
Member

Didn't even notice, sure they fired when I pushed it earlier.

@davidzwa
Copy link
Contributor Author

If you could, ##386 fits nicely in this PR

@NotExpectedYet
Copy link
Member

Yeah no worries will sort it in the morning.

@NotExpectedYet
Copy link
Member

Was because of the merge, all firing now.

CHANGELOG.md Outdated Show resolved Hide resolved
checkForNamedService = true;
}
if (isNodemon()) {
exec("touch ./app.js").catch((stderr) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Smart man! See what u did there.

Copy link
Member

Choose a reason for hiding this comment

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

:P

Copy link
Member

@NotExpectedYet NotExpectedYet left a comment

Choose a reason for hiding this comment

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

After much pain and strife!

@NotExpectedYet NotExpectedYet merged commit ff018ad into development Apr 18, 2021
1.1.13 - Pushup Party automation moved this from Awaiting Review to Done Apr 18, 2021
@NotExpectedYet NotExpectedYet deleted the feature/update-octofarm branch April 18, 2021 14:47
NotExpectedYet added a commit that referenced this pull request Apr 24, 2021
* Squash and rebase work on updater

* Autoformat and fix disabled not bool bug

* Fix forcecheck not a function

* Fix

* fix up to date catch

* Moved up-to-date as last check and added "Changes not stages for commit" grab

* If cancel update button disabled = false

* update changelog

* update changelog

* Gulpy fish

* remove git exec catches for stderr

* message fix, and send the correct boolean you lord of the toilet rolls

* Fix syntax error

* False

* Fake commit for testing pull

* removal of the unneeded checkeroo

* Fake commit again

* And again

* removal of the unneeded checkeroo

* server reponse

* Fake commit

* Fix my wild goose chase bloody toilet roll failed me!

* Fake commit

* fix for git reset changes

Co-authored-by: NotExpectedYet <online@notexpectedyet.com>
Co-authored-by: James Mackay <info@notexpectedyet.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants