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

Rework the update process #155

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

emilecantin
Copy link
Contributor

This changes the update process so the Python code itself calls the
unzip utility (instead of going through a script). The new code is also
extracted to a temporary directory which is then moved into place after
the current version has been modev away as a backup. I also added a lot
of logging around this area, to have better visibility on what happens.

This changes the update process so the Python code itself calls the
unzip utility (instead of going through a script). The new code is also
extracted to a temporary directory which is then moved into place after
the current version has been modev away as a backup. I also added a lot
of logging around this area, to have better visibility on what happens.
@emilecantin emilecantin mentioned this pull request May 28, 2020
@emilecantin
Copy link
Contributor Author

Just note; I couldn't test on Windows, though.

@madgrizzle
Copy link
Collaborator

How do you go about testing this? Just trying to figure out how to do it without really messing something up.

@emilecantin
Copy link
Contributor Author

emilecantin commented May 28, 2020 via email

@madgrizzle
Copy link
Collaborator

Just delete the new version and move back your git copy to undo.

This is the part I'm confused about as I'm not a git expert and I've royally screwed things up in the past. If I check out the update_process branch, run it and do an upgrade, can I just then checkout the master branch again to revert back and not have any worries?

@emilecantin
Copy link
Contributor Author

Yes, but the update process will actually move / rename the folder you're working from (it'll add _old at the end). To undo that, you'll need to rename it to what it was originally; your git stuff will be untouched.

@madgrizzle
Copy link
Collaborator

I ran the branch and it appeared to do the update as you indicated, but it didn't restart webcontrol after shutting down.

@emilecantin
Copy link
Contributor Author

emilecantin commented May 28, 2020

That's expected. A typical headless install would have WebControl run as a service, and the service manager (systemd on most Linuxes) would see the process stop and attempt to restart automatically. That's why I moved the program exit to the very end of the upgrade process. The auto-restart we had before would cause some issues with that kind of setup.

A non-headless install would be very easy for the user to restart on their own. I think there should probably be a UI component with this, though, but I haven't had the time to dig into it yet. I just wanted a "sanity check" that the process itself works properly before investing time into that.

@gb0101010101
Copy link
Contributor

Sorry all - I somehow messed up twice on adding a change to this PR.

First I accidentally created branch upstream/feature/update_process when I trying to push a change to branch feature/update_process. The change there is correct but doesn't need to be in a new branch. I wanted it here so that this one can be tested with it.

Then I tried again uploading the file directly using UI but somehow the change is different in commit e90e9de. So this one should certainly be deleted.

I do not have permission to delete the branch I created or remove the commit here. Can someone do it form me? Please make sure not to delete branch feature/update_process.

@emilecantin
Copy link
Contributor Author

@gb0101010101 I just deleted it. Thank you for this change, the error was a bit annoying. However, I wasn't really planning on keeping date-formatted releases around; they were just a quick an easy way to generate unique tags to mark my test releases. Actual releases would still use the same format.

Also, you undid my changes to the experimental release detection. We discussed these changes early on in #139, and all the experimental releases are correctly marked in Github as such; no need to parse the version number anymore.

@emilecantin
Copy link
Contributor Author

Just force-pushed to remove the commit from this branch.

@gb0101010101
Copy link
Contributor

Would it make sense to keep Tag and add date, e.g. 0.94_2020-05-30, as the release name. Currently cannot compare to running release.

Also should we create a webcontrol.version file (JSON/YML) that can be auto created during build so that source code does not need to be manually changed to bump version numbers.
This could also have:

  • GitHub project path
  • Docker image paths
  • Date for use as URL parameters for loading JS and CSS

@emilecantin
Copy link
Contributor Author

emilecantin commented May 30, 2020 via email

@davidelang
Copy link

davidelang commented May 30, 2020 via email

@gb0101010101
Copy link
Contributor

gb0101010101 commented May 30, 2020

Remember on windows, you can't access/overwrite a file if it's open.

The version file would be opened, read, and closed on WebControl startup. Once read the version info would be used from memory. So this should not be a problem.

… have been fetched from github. Use regex to match: stable having only version number with/without 'v' prefix, pre-release having version number and date, with/without 'v' prefix using underscore and date format YYYY-MM-DD. Use full release.tag_name when performing update to ensure it uses the right release.
@emilecantin
Copy link
Contributor Author

No, he's talking about moving the current release to old; can't have a running python file move itself. However, that's the very last thing that happens, and it's pretty quick. I can make it be a different script on Windows. The main goal of this work is to prepare as much of the update as we can in advance, before doing the actual move; and also to keep the 2 versions as separate as possible.

@gb0101010101
Copy link
Contributor

@emilecantin Can you help me with adding commits to this PR so I don't mess things up again. I want my commits to be part of this PR but be reviewed before they are accepted.

Using Git CLI I have git remote -v

origin  [email protected]:gb0101010101/WebControl.git (fetch)
origin  [email protected]:gb0101010101/WebControl.git (push)
upstream  [email protected]:WebControlCNC/WebControl.git (fetch)
upstream  [email protected]:WebControlCNC/WebControl.git (push)

I did:

git checkout master
git fetch upstream
git checkout -b feature/update_process upstream/feature/update_process

Have local feature/update_process branch with your changes. Tested, made changes, and commits. Now want to push changes to this PR #155 so they will be grouped with your changes and added to branch feature/update_process. I don't want my changes committed until they have been reviewed together with your changes.

Should I push feature/update_process branch to my GitHub fork and create a new Pull Request to this repo? Or should I push upstream feature/update_process to this repo directly.

Previously I did:

git checkout -b upstream/feature/update_process
# Created local upstream/feature/update_process
# Made changes and commits
git push upstream upstream/feature/update_process

This created branch upstream/feature/update_process on this repo which was wrong.

Thanks for any help on this.

@gb0101010101
Copy link
Contributor

No, he's talking about moving the current release to old; can't have a running python file move itself.

Sorry for missing something here. I plan on adding a file that acts like defaultwebcontrol.json that contains release info. Can defaultwebcontrol.json be overwritten on Windows whilst WebControl is running? Have not tested, so do not know, but thought this is not considered a running file.

@emilecantin
Copy link
Contributor Author

emilecantin commented May 31, 2020

@gb0101010101 You have write access to this repo, you don't need to use your own fork to do changes. You can add this repo as origin directly; it'll be a lot simpler this way. What you can do is checkout my branch (feature/update_process), create and checkout a new branch from there (git checkout -b blahblah-whatever), do your changes on that branch as you wish, and finally open a PR with that branch against mine (see screenshot)

Capture d’écran, le 2020-05-31 à 10 07 44

@davidelang
Copy link

davidelang commented May 31, 2020 via email

@emilecantin
Copy link
Contributor Author

emilecantin commented May 31, 2020 via email

@gb0101010101
Copy link
Contributor

OK I was able to keep my current setup and push using:
git push upstream feature/update_process
Accidentally added some hidden files which have now been removed.

Main changes are:

  • Added pyInstallCurrentVersionDate to track date of running release
  • Renamed pyInstallCurrentVersion to pyInstallCurrentVersionNumber to track running version number.
  • Added regex to parse github tag name and split it into version number and version date. This will match patterns
0.94
v0.94
0.94_2020-05-30
v0.94_2020-05-30

Any tag name not matched is ignored and release is not listed in WebControl

  • Added comparison logic to check for latest release. First compares version number then version date.
  • Change template to use version number and version date
  • Made sure that if a function is called it will always have the data it needs to return release info.

This has some test code to make the test release 2020-05-26-2021 work which should be removed.

@emilecantin
Copy link
Contributor Author

@gb0101010101 Thank you, these changes look pretty good. Altough I'd probably go even simpler and not even try to parse the version number; instead relying on the /latest endpoint to get the latest release. Anyway, it looks like the code you have right now is sufficiently flexible to handle many use-cases.

@madgrizzle and @davidelang How do you typically run WebControl on Windows? Do you create a service? If so, does it restart WebControl automatically when it quits? Asking this because I'll do the folder move in a script that runs after WC quits, and I'm wondering if it should restart the new version once it's done.

@davidelang
Copy link

davidelang commented Jun 1, 2020 via email

@madgrizzle
Copy link
Collaborator

I don't use Windows either. I would assume if you are running windows, you are not doing it headless and would double-click a shortcut or something to start it.

@emilecantin
Copy link
Contributor Author

Okay, I was really under the impression that both of you ran Windows, haha. Any Windows users care to comment? Otherwise I'll go ask on the forums.

@gb0101010101
Copy link
Contributor

FYI I am working on this. I think we can restart using python os.execl().
Making an Update modal with status updates and download progress bar.
I have a multiboot with Win10 (for games) and VM Win7 64 so will be able to test.

@emilecantin
Copy link
Contributor Author

Honestly, I'd prefer if we didn't restart the server; I'd leave that to the user (or whatever they use to run WC).

@gb0101010101
Copy link
Contributor

I am adding new platform detection. May be able to detect if its running headless and only restart for this case. Just to be clear this will not reboot the device; it will only restart the WebControl process.

@emilecantin
Copy link
Contributor Author

emilecantin commented Jun 1, 2020

Restarting the server will break the headless Raspberry Pi setup I have on the "official" image. That's sytemd's or the user's job.

So we especially should not restart when running headless.

@davidelang
Copy link

davidelang commented Jun 2, 2020 via email

@gb0101010101
Copy link
Contributor

how do you replace the running binary without a restart on windows?

@davidelang Don't know yet but there must be an existing process.

Restarting the server will break the headless Raspberry Pi setup I have on the "official" image.

@emilecantin Can you quickly explain why?

@emilecantin
Copy link
Contributor Author

@davidelang I'm not talking about leaving running. I'm talking about simply exiting once the update is done, and let the user (or service manager) restart.

@gb0101010101 On Raspberry Pi, systemd is watching the process, so if it quits, it'll restart it automatically. In this case, simply exiting once the update is done will trigger systemd to re-start it, and since the new version is now at the path that systemd is looking at, it'll launch the new version automatically. Having something else try to launch it at the same time will cause issues (port in use, files opened twice, that sort of stuff).

I suspect anything running WebControl in a headless installation will have a similar behaviour. That's why I don't want us to try to restart the new version at the end of the update; let whatever started WebControl (user or service manager) handle it.

@davidelang
Copy link

davidelang commented Jun 2, 2020 via email

@emilecantin
Copy link
Contributor Author

@davidelang I know, but it (systemd with auto-restart) is a common option, and should probably be the recommended one for Linux. MacOS uses launchd which is very similar, and Windows also has a similar functionality. I know some Linux distros haven't adopted systemd, but any init system worth using would have an option to restart stopped / crashed services.

I just asked on the forums how Windows users run their WC, just to get a sense of the common usage patterns; it'll help us make a better decision.

@gb0101010101
Copy link
Contributor

Sorry this took so long. Had many issues because things that should work did not.
Still WIP but thought I would share now as I don't know how much time in the coming week.

NOTES

  • Do not run update in your development environment as it will overwrite files. A backup is created but its still a pain to revert. If you want to test in dev then comment out the two os.rename() commands at the end of updatePyInstaller.
  • When debugging the socket.io/serial communication will break due to hardcoded existing timeout. I think its 5 seconds. So set watches rather then breakpoints.

Working

  • Click "Upgrade" button and modal changes to display update progress.
  • Modal footer, 'X' button, and click outside are disabled to stop accidental interruption of upgrade progress.
  • Uses separate socket.io "modal" room to communicate so its isolated from main communication.
  • Status messages updated with progress text
  • Progress bar showing download status
  • Existing code by @emilecantin that performs update works.
  • Re-enable Modal footer, 'X' button, and click outside to close modal when update succeeds.
  • Only shutdown when update succeeds.
  • Report upgrade errors.

Not Working or partial/wrong

  • I kludged the disabling of "click outside modal" because I could not get the proper Bootstrap way to work. I think this is because the modal exists in base.html when it really should not.
  • Really pretty much the whole of WebControl should be disabled before the upgrade process starts.
  • Wanted to add service to restart WebControl but ran out of time.
  • DataStructures/data.py defines pyInstallPlatform = "win" and pyInstallType = "singlefile" in repo. This should be detected for repo downloaded files and made set in new file webcontrol.version for package releases. This file should be auto generated when package is created. Again ran out of time.

So these changes only add new features to this branch but I don't think this can be current considered fully working.

Please let me know you thoughts/concerns. I will certainly be reading messages but probably won't be able to change anything in the next week.

@gb0101010101
Copy link
Contributor

@madgrizzle I thought that uiQueue would pass socket events to browser but could not get this to work reliably. I used socketio.emit directly instead. If you know of a better way then let me know.

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.

4 participants