Feat : Add download update feature in user app. Issue #755

[shashank40]

/claim #755

What kind of change does this PR introduce?

Feature addition

Summary

Currently we don't support app update functionality in our app. With this addition, user can get the latest version of app from the app-tray itself.

1. App shows latest version to download if available
2. Else it shows no update found.

Video

https://github.com/OpenAdaptAI/OpenAdapt/assets/48802836/d3058697-5e03-4f9a-ad4a-8cb33930fe87

Checklist

• [ ] My code follows the style guidelines of OpenAdapt
• [ ] I have performed a self-review of my code
• [ ] If applicable, I have added tests to prove my fix is functional/effective
• [ ] I have linted my code locally prior to submission
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
• [ ] New and existing unit tests pass locally with my changes

How can your code be run and tested?

Other information

[shashank40]

Hi @abrichr I have implemented the feature to allow the user to download the latest app when needed.

Now why should we do this and not automatic update? As this is an Industry standard.

No app downloads the updates on every start as :

1. This makes the app slow
2. We are forcing an update on the user.
3. User needs to have multiple command line tools to build it on the go. This is not at all ideal.

So this comment was the best solution and i have implemented that.

I have also added a video in the description showing how the feature works.

[abrichr]

@shashank40 this a great start!

In the comment you linked, the third step is:

On clicking that, the newest version should be downloaded, and unzipped in the same directory as where the current app lives. There are no additional installation instructions for OpenAdapt, so we could just display a message box saying that the new app was downloaded, and maybe delete the current version as well (@abrichr thoughts on this as well)

If you implement this we can consider this. Keep going!

[shashank40]

@abrichr Just a thought! According to me unzipping in the download directory better than current directory as people generally look for downloads in the downloads folder rather than any random directory)? (Your thoughts @abrichr?)

I think that is the only part left in the comment.

But i will implement the unzipping part for sure.

[shashank40]

Also @abrichr i dont think we should implement deleting the old version. Reason : what if the user is in a middle of the recording during download? And we delete this old build, this is not ideal

I think extracting the zip is the best thing we should do and not the deleting part.

Thoughts? @abrichr

[abrichr]

@shashank40 typically how applications handle this is to say something like:

A new version is available: vxx.xx.xx (you are on vyy.yy.yy). Would you like to update? Yes/No/Skip this version

We can show a message on startup indicating that a new version is available, and let the user decide how to proceed.

[shashank40]

@abrichr Yess i agree, but isn't this similar to what is implemented but better?. Rather than popup we have it in a tray with the version to update visible. User can choose to implement and not to implement.

Why i say this is, as OpenAdapt gets latest updates quite frequently(sometimes even 2-3 a day) and not weekly/monthly. So let's say i update it once and then few hours later there would be another popup hindering me.

This would be a bad user experience every time i open an app.

The implementation that you suggested might work better for apps/softwares that have scheduled updates that come weekly/monthly. But with these frequent updates, our user experience will go down.

[abrichr]

It's an interesting point @shashank40. But I still think we want to make the user's life easier by automatically extracting the application for them if they do choose to update.

[shashank40]

@abrichr You mean extracting the zip file?

[shashank40]

@abrichr Done. Added the functionality to unzip as well

[abrichr]

@shashank40 I'm unable to test this on my machine because of https://github.com/OpenAdaptAI/OpenAdapt/issues/785.

Can you please record a video showing the updated functionality?

[abrichr]

@shashank40 if you can upload a video showing this working, address my comments, and fix the formatting issues causing the build to fail, then I think we can get this merged!

[shashank40]

Surely @abrichr , will update this in sometime. Currently AFK.

[shashank40]

Hi @abrichr.

I have replied back to the comments. I am also adding a new video here.

In the video you can see the latest version now shows as 0.34.0 and in yesterday's video it was 0.33.2 and so this makes the app quite reactive to new updates. That will be the beauty of this new update. Dynamically telling you the latest version available.

This is a full video of download process so from start till 3:30 mins it mostly shows download > then it shows unzipping > then it shows the unzipped new version available for the user.

https://github.com/OpenAdaptAI/OpenAdapt/assets/48802836/c9a4cd16-44a1-4ad2-9aa9-c5a5ac8421b0

[shashank40]

Hey @abrichr can we go ahead and merge this?

[shashank40]

@abrichr any update on this?

[abrichr]

Hi @shashank40 , this looks good. However it's unfortunate that there is no visual indicator for the user about the progress, other than what is in the terminal (users who run the app not from the terminal won't see any progress).

I think we need some sort of status indicator, e.g. via show_toast. What do you think?

[shashank40]

Yess @abrichr , I can add a toast after the zipping is complete. Will do it in right now.

[shashank40]

@abrichr As the video would get too long to upload, i am adding 2 parts of the video. Hope this is fine?

1) Shows how the toast would popup with approximate time to download.

https://github.com/OpenAdaptAI/OpenAdapt/assets/48802836/939da167-3ec2-4d8f-a6ff-4aa293072655

2) This video skips the whole download process as that makes the video too long. It shows the end part when download was completed and we show a toast telling the user that download is complete.

https://github.com/OpenAdaptAI/OpenAdapt/assets/48802836/a47d0708-4458-4563-a9f5-e185acf2187e

[abrichr]

@shashank40 thank you for the quick turnaround! This is not bad but I think we can do better.

Can you please add a callback that clears the current toast, and adds a new one with an updated ETA?

[shashank40]

@abrichr I initially did that, but as our main thread will be busy doing much more important things like recording/visualize, making main thread do regular UI upgrades(every few milliseconds as UI changes can only be done by main thread) is degrading performance as main thread chokes.

I am okay with updating the time every few seconds, but i think giving an approximate at the ends works okay and does not hinder with more important tasks

[abrichr]

No need to update every few milliseconds, but I believe every e.g. 1s would be much more usable at minimal performance cost. 🙏

[shashank40]

@abrichr Okay, so i will do a new toast pop-up every 2 secs with updated time estimate? Will that be fine? or is it something else you might want?

[abrichr]

At the same time be sure to clear the previous toast so that we don't ever have more than one toast showing regarding the update progress. Ideally it looks like the same toast, so remove any animations etc. Make sure the update period (e.g. 2s) is configurable.

[shashank40]

Yess @abrichr i made sure of that. Here is the video. Hopefully now we can get it merged. It's been too long now 😅

https://github.com/OpenAdaptAI/OpenAdapt/assets/48802836/68819acd-b467-43a9-9f1d-3982082d3209

[abrichr]

Hi @shashank40 this is looking good! I haven't merged it yet because I saw you pushed a few commits after your last comment. Please fix the merge conflicts and let me know when this is ready for re-review, would love to get this merged! 🙏

[shashank40]

@abrichr I did all the required changes. Removed the conflict. Hopefully this now looks all good.

[shashank40]

@abrichr Any update?

[abrichr]

Hi @shashank40 , thank you for your patience,

I tried running this on my machine:

open_adapt-v0.36.0.app.zip:  31%|██████████████████████████████▍                                                                   | 9.00/29.0 [00:00<00:00, 75.8kB/s]
Exception in thread Thread-5 (download_latest_version):
Traceback (most recent call last):
  File "/opt/homebrew/Cellar/python@3.10/3.10.14/Frameworks/Python.framework/Versions/3.10/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/opt/homebrew/Cellar/python@3.10/3.10.14/Frameworks/Python.framework/Versions/3.10/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/abrichr/oa/OpenAdapt/openadapt/app/tray.py", line 297, in download_latest_version
    unzip_file(local_filename)
  File "/Users/abrichr/oa/OpenAdapt/openadapt/update_utils.py", line 19, in unzip_file
    shutil.unpack_archive(file_path, os.path.dirname(file_path))
  File "/opt/homebrew/Cellar/python@3.10/3.10.14/Frameworks/Python.framework/Versions/3.10/lib/python3.10/shutil.py", line 1314, in unpack_archive
    func(filename, extract_dir, **kwargs)
  File "/opt/homebrew/Cellar/python@3.10/3.10.14/Frameworks/Python.framework/Versions/3.10/lib/python3.10/shutil.py", line 1201, in _unpack_zipfile
    raise ReadError("%s is not a zip file" % filename)
shutil.ReadError: /Users/abrichr/Downloads/open_adapt-v0.36.0.app.zip is not a zip file

It appears that unzip_file is being called before the download completes.

Any suggestions?

Edit: at a minimum we should wrap this in a try/except and warn the user with a toast if the update fails.

[shashank40]

@abrichr Any more changes that might expect ? I would complete it today so that we can get this pending PR in.

[abrichr]

Hi @shashank40 I just noticed I introduced the latest bug with my changes 😅 will fix.

Edit:

OpenAdapt-v0.36.0.app.zip: 100%|█████████████████████████████████████████████████████████████████████████████████████████████████| 2.03G/2.03G [03:20<00:00, 10.1MB/s]
Exception in thread Thread-5 (download_latest_version):
Traceback (most recent call last):
  File "/opt/homebrew/Cellar/python@3.10/3.10.14/Frameworks/Python.framework/Versions/3.10/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/opt/homebrew/Cellar/python@3.10/3.10.14/Frameworks/Python.framework/Versions/3.10/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/abrichr/oa/OpenAdapt/openadapt/app/tray.py", line 297, in download_latest_version
    unzip_file(local_filename)
  File "/Users/abrichr/oa/OpenAdapt/openadapt/update_utils.py", line 20, in unzip_file
    set_permissions(os.path.dirname(file_path))
  File "/Users/abrichr/oa/OpenAdapt/openadapt/update_utils.py", line 11, in set_permissions
    os.chmod(os.path.join(root, dir), 0o755)
PermissionError: [Errno 1] Operation not permitted: '/Users/abrichr/Downloads/<redacted>/dosdevices/z:'

I think we need to wrap everything in a try/except as described previously. Ideally we also:

• notify the user when extraction is taking place
• fix this permission issue

[shashank40]

Yes @abrichr i saw that. It was actually working fine for me till that commit. It was the file name changed to open_adapt.

Even now works fine for me

[shashank40]

i am adding a try catch to the code.

Also @abrichr this is a permission issue specific to the file on your mac. We can skip files like these in try catch and it would work fine. Some files need administrative permissions to change the perms.

I am adding that as well. Things should work fine now.

[shashank40]

@abrichr I have added started unzipping toast. I have also added the check if i am on the latest version, menu action will be disabled.

[shashank40]

@abrichr Please have a look at the latest code.

[shashank40]

Hi @abrichr Made all the specified changes. I hope its better.

[shashank40]

@abrichr any update?

[shashank40]

@abrichr Can you please do a final review?

[abrichr]

Thank you @shashank40. There appear to be a few loose ends:

1. The "extracting" toast disappears, and there is no indication to the user that anything is happening. Ideally we show progress on this as well, e.g. using the zip_file module:

def extract_with_progress_zip(zip_path: str, extract_to: str):
    with zipfile.ZipFile(zip_path, 'r') as zip_ref:
        total_files = len(zip_ref.infolist())
        for i, file in enumerate(zip_ref.infolist()):
            zip_ref.extract(file, extract_to)
            logger.info(f"Extracted {i + 1} of {total_files} files ({(i + 1) / total_files * 100:.2f}%)")
            # TODO: update toast, like with downloading

2. Once unzipping is complete, the user will have more than one copy of OpenAdapt on their hard drive. Ideally we: a. Move the newly extracted version to the same directory as the old version. b. Ask the user whether they would like to remove the previous version, and if so then remove it*.

3. Finally, I would expect that: a. the currently running version of OpenAdapt is shut down b. the newly extracted version's executable is executed

• The removing will probably need to be done on the first launch of the new version.

I would like to merge this now, but as it currently stands, it's not very user friendly without these additions, i.e. once the extraction is complete, the user likely expects to be running the new version, even though currently they won't be.

Once these additions have been implemented, the update feature will be complete and we can merge this.

Thank you @shashank40 , almost there!

Edit: implementation, courtesy of ChatGPT:

---

import os
import sys
import shutil
import subprocess
import atexit
from PySide6.QtWidgets import QMessageBox, QApplication

def get_current_executable_path() -> str:
    if hasattr(sys, '_MEIPASS'):
        # If running from a PyInstaller bundle
        return sys.executable
    else:
        # If running as a script
        return os.path.abspath(__file__)

def get_target_path() -> str:
    current_executable_path = get_current_executable_path()
    return os.path.dirname(current_executable_path)

def move_new_version(extracted_path: str, target_path: str) -> bool:
    try:
        # Move contents of the new version to the target path
        for item in os.listdir(extracted_path):
            s = os.path.join(extracted_path, item)
            d = os.path.join(target_path, item)
            if os.path.isdir(s):
                shutil.copytree(s, d, dirs_exist_ok=True)
            else:
                shutil.copy2(s, d)
        return True
    except Exception as e:
        print(f"Error moving the new version: {e}")
        return False

def ask_user_to_remove_old_version() -> bool:
    app = QApplication([])
    msg_box = QMessageBox()
    msg_box.setText("Would you like to remove the previous version of OpenAdapt?")
    msg_box.setStandardButtons(QMessageBox.Yes | QMessageBox.No)
    reply = msg_box.exec()

    if reply == QMessageBox.Yes:
        return True
    else:
        return False

def remove_old_version(old_version_path: str):
    try:
        shutil.rmtree(old_version_path)
        print("Old version removed successfully.")
    except Exception as e:
        print(f"Error removing old version: {e}")

def shutdown_current_version():
    QApplication.quit()
    sys.exit(0)

def launch_new_version(new_executable_path: str):
    def launch():
        try:
            subprocess.Popen([new_executable_path])
        except Exception as e:
            print(f"Error launching the new version: {e}")

    atexit.register(launch)

def main():
    # Paths for the extracted new version and target location
    extracted_path = "/path/to/extracted/new/version"
    target_path = get_target_path()
    new_executable_path = os.path.join(target_path, "openadapt_executable")

    # Move the new version to the target location
    if move_new_version(extracted_path, target_path):
        # Ask the user if they want to remove the old version
        if ask_user_to_remove_old_version():
            # On the first launch of the new version, remove the old version
            remove_old_version(target_path)

        # Schedule the new version to launch after shutdown
        launch_new_version(new_executable_path)

        # Shut down the current version
        shutdown_current_version()

if __name__ == "__main__":
    main()

[shashank40]

@abrichr I think we should not do part 1), reason is extraction takes 5-10secs. Now we already show toast when it starts and when extraction completes. The overhead and resource consumption to show extraction time left(which in turn is shown by 2 different toast) seems excessive.

For 2, again i am less aligned. The things we are trying to implement happens in installed apps and not desktop apps which OpenAdapt is. I think we should compare OpenAdapt to Desktop Apps and not AppStore applications. In desktop apps, if we download 10 different ZIP files which have 10 different versions , they all stay and is manually deleted by the user if needed. A zip/unzipped file is never deleted by an app as this is being intrusive. It is always the decision of a user to delete the the file he downloaded manually. There are many reasons for it :

1. This will have strict permission requests from OS and can mark the app dangerous, specially for mac because if the app gets the permission to delete one file, it can be directed to delete any number of files from a personal computer, which is dangerous.
2. I have never seen desktop apps deleting manually downloaded files and we should stick to the norm. If a user is downloading something, we should let the user decide what he wants to do with that.
3. If the user selects yes on delete the old version and the new version is faulty, he won't be able to download the old version again, which again ruins the user experience.
4. Also deleting a file automatically is not at all enhancing the user experience as compared to the intrusion its causing.

Eventually its your take, but i would never install an app that can delete files from my personal computer. I would rather choose to delete a file manually(as it literally takes 1/2 a sec) than installing an app that can delete files.

[abrichr]

Thank you @shashank40 , I appreciate you presenting an alternative perspective.

Regarding extracting the zip file: on my machine, it can take a while. The notification disappears before the extraction is complete, then it's not clear whether the process is complete or not. I think it's important we implement similar logic as for downloading to indicate the update is still occurring.

Regarding not removing the old application, the intention here is to emulate the behavior of other applications that update automatically. For example, when I update Chrome, I don't have to remove the old directory. What do you think?

[shashank40]

@abrichr When comparing it to chrome, browsers are installed applications which have their own directory on the hard-drive and change required files when updated. So they never touch any other directory/downloaded file. Even if we download a multiple zips for chrome, they are never deleted by chrome.

So installed apps work very differently in how updates work and how files are changed in their installed directory locations.

OpenAdapt is a desktop app and not and installed app. It doesn't have a fixed directory like installed apps and so deleting files from any other directory needs higher level permissions and makes it unsafe. If OpenAdapt can delete files from any directory, it makes it unsafe.

[shashank40]

@abrichr updates?

[abrichr]

Hi @shashank40 , thank you for your patience.

OpenAdapt is a desktop app and not and installed app.

We've implemented installers in https://github.com/OpenAdaptAI/OpenAdapt/pull/858. We would like the update functionality to work with the installed app.

Edit: I know you're been at this one for a while. Almost there!