pre launch hook for installing AYON extension #17
Conversation
This makes me wonder - we do have permissions most likely to read from that location? And if so, we could basically validate the existence from the hook too, and potentially log a warning? |
BigRoy
added
the
type: enhancement
Good shout! we'd need to update Prem impl as well, but we would first need to confirm that each of these extension managers is installing to the exact same locations and using the same method to store so it might be worth throwing this up as an issue and addressing later? |
Yes, fine with me. 👍 |
client/ayon_aftereffects/hooks/pre_launch_install_ayon_extension.py
Outdated
Show resolved
Hide resolved
| title="Templated Workfile Build Settings" | ||
| title="Templated Workfile Build Settings", | ||
| ) | ||
| hooks: HooksModel = SettingsField( |
There was a problem hiding this comment.
I'm a little bit late, sorry, but does this have to be in hooks/InstallAyonExtensionToAfterEffects? I would rather see it above all plugins as simple checkbox (something like auto_install_extension). The path in settings does not have to match where it is used...
There was a problem hiding this comment.
Not at all, this is a direct port of ynput/ayon-premiere#6 which didn't have any settings section prior to that. Again I based this PR on existing examples of pre-launch hooks so just went with mirroring the approach example. I don't have a strong preference of where it should live.
There was a problem hiding this comment.
It's not wrong, it can be as is, it was more UX question. Other opinions, e.g. @kalisp ?
| new_version = ( | ||
| ET.parse(xml_file) | ||
| .getroot() | ||
| .attrib.get("ExtensionBundleVersion") | ||
| ) | ||
| if not new_version: | ||
| self.log.warning( | ||
| "Unable to resolve the new extension version. " | ||
| "Cancelling.." | ||
| ) | ||
| self.log.debug(f"New extension version found: {new_version}") | ||
|
|
||
| # compare the two versions, a simple == is enough since | ||
| # we don't care if the version increments or decrements | ||
| # if they match nothing happens. | ||
| if installed_version == new_version: | ||
| self.log.info("Versions matched. Cancelling..") | ||
| return False | ||
|
|
||
| # remove the existing addon | ||
| self.log.info( | ||
| "Version mismatch found. Removing old extensions.." | ||
| ) | ||
| rmtree(target_path) | ||
| return True |
There was a problem hiding this comment.
I think we don't need to have all the code under with open context manager?
| new_version = ( | |
| ET.parse(xml_file) | |
| .getroot() | |
| .attrib.get("ExtensionBundleVersion") | |
| ) | |
| if not new_version: | |
| self.log.warning( | |
| "Unable to resolve the new extension version. " | |
| "Cancelling.." | |
| ) | |
| self.log.debug(f"New extension version found: {new_version}") | |
| # compare the two versions, a simple == is enough since | |
| # we don't care if the version increments or decrements | |
| # if they match nothing happens. | |
| if installed_version == new_version: | |
| self.log.info("Versions matched. Cancelling..") | |
| return False | |
| # remove the existing addon | |
| self.log.info( | |
| "Version mismatch found. Removing old extensions.." | |
| ) | |
| rmtree(target_path) | |
| return True | |
| new_version = ( | |
| ET.parse(xml_file) | |
| .getroot() | |
| .attrib.get("ExtensionBundleVersion") | |
| ) | |
| if not new_version: | |
| self.log.warning( | |
| "Unable to resolve the new extension version. " | |
| "Cancelling.." | |
| ) | |
| self.log.debug(f"New extension version found: {new_version}") | |
| # compare the two versions, a simple == is enough since | |
| # we don't care if the version increments or decrements | |
| # if they match nothing happens. | |
| if installed_version == new_version: | |
| self.log.info("Versions matched. Cancelling..") | |
| return False | |
| # remove the existing addon | |
| self.log.info( | |
| "Version mismatch found. Removing old extensions.." | |
| ) | |
| rmtree(target_path) | |
| return True |
There was a problem hiding this comment.
Good spot! I won't have a chance to validate this until next week. It looks correct so happy to commit if you've tested, if not I'll test asap and commit
| rmtree(target_path) | ||
| return True | ||
|
|
||
| except PermissionError as error: |
There was a problem hiding this comment.
I would get rid of try -> except blocks here. It is captured in the execute also printing traceback (giving more information if something happens).
| ): | ||
| return | ||
|
|
||
| try: |
There was a problem hiding this comment.
I would get rid of try -> except blocks here. It is captured in the execute also printing traceback (giving more information if something happens).
client/ayon_aftereffects/hooks/pre_launch_install_ayon_extension.py
Outdated
Show resolved
Hide resolved
client/ayon_aftereffects/hooks/pre_launch_install_ayon_extension.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
Changelog Description
A prelaunch hook that automatically installs the AYON extension to the users appdata (and will update it if version mismatch found)
Additional review information
A port of: ynput/ayon-premiere#6, more information can be found there.
IMPORTANT: If you have already installed the AYON AfterEffects extension to users you will need to remove this via the tool you used (either ExManCmd or Anastasiy's Extension Manager) since they are located in system paths we are unable to remove these due to permissions. You will not experience any error by having the extension in both paths BUT its a gamble on which one it's actually loading. This is also Windows only for now and would require a Mac user to do the relevant R&D.
Testing notes: