ynput / ayon-shotgrid

A Shotgrid Addon for the Ayon server.
Apache License 2.0
5 stars 8 forks source link

Create package script functionality with Upload arg #84

Closed jakubjezek001 closed 5 months ago

jakubjezek001 commented 5 months ago

Since server settings were upgraded one need to update addon at your ayon-docker/addons folder. In repository root create and copy addon package to your local ayon-docker (this is still old structure so you should not clone this repo to addons folder directly). Or do it with the ZIP file uploading if you want to waist your time 😄 , or docker with server is not running from local machine.

python .\create_package.py --skip-zip --output [PATH TO YOUR REPO]\ayon-docker\addon

I see, I think the zip upload is actually easier to understand if you are not running a local ayon docker and it actually is easier to follow and really fast! But ideally the same create_package would have an --upload flag taking the local .env so it uploads the zip directly to the server like there is for the launcher

Originally posted by @fabiaserra in https://github.com/ynput/ayon-shotgrid/issues/83#issuecomment-2043164765

jakubjezek001 commented 5 months ago

I am assuming that the complexity of the create package script would be then much higher, since we would need to deal with rest api points. But that could probably be done with simple requests build in dependencies, right @iLLiCiTiT ?

iLLiCiTiT commented 5 months ago

I would say this feature would be for development, and for development you ideally should use local server. The new structure of addon actually supports to use the repository directly (not sure if that will be possible with shotgrid as it might have some common code copied during create package logic).

I personally don't think it should be there as it adds dependency requirement for create package script. The point of create_package.py script was that you can run it without any pypi dependencies (so you can easily automate it).

But that could probably be done with simple requests build in dependencies, right @iLLiCiTiT ?

Well, if you require requests you can also require ayon_api.

fabiaserra commented 5 months ago

I would say this feature would be for development, and for development you ideally should use local server. The new structure of addon actually supports to use the repository directly (not sure if that will be possible with shotgrid as it might have some common code copied during create package logic).

I use the local repo directly for client code yeah, I only had to create a symlink for the version.py file and for the services I just rebuild and run the docker containers locally every change I do, which does the copy of the common code on the Dockerfile. Are you saying the services can be also ran directly from source?

iLLiCiTiT commented 5 months ago

Are you saying the services can be also ran directly from source?

You would have to implement it. e.g. in ftrack there are services and services tools, the tools run the same code which services do in docker, but localy.

jakubjezek001 commented 5 months ago

Lets close this issue, since ShotGrid addon can be now cloned into ayon-docker/addons and since 0.4.0 and there is no need uploading argument since as @iLLiCiTiT suggested it is recommended to work on local docker containers in develop mode.

fabiaserra commented 5 months ago

Lets close this issue, since ShotGrid addon can be now cloned into ayon-docker/addons and since 0.4.0 and there is no need uploading argument since as @iLLiCiTiT suggested it is recommended to work on local docker containers in develop mode.

Regardless of develop mode, it's a useful feature for deploying to the production server... as you need to run create_package when you are ready to deploy to production

fabiaserra commented 5 months ago

Kitsu addon already does this https://github.com/ynput/ayon-kitsu/blob/develop/create_package.py#L519 ?

iLLiCiTiT commented 5 months ago

Kitsu addon already does this https://github.com/ynput/ayon-kitsu/blob/develop/create_package.py#L519 ?

And does it make sense to upload it from code to a server? Only time I would like to do it is for development, never for production. But you can add the addon repository to server addons directory for live development, so I don't see any advantage in that.

fabiaserra commented 5 months ago

And does it make sense to upload it from code to a server?

Of course it makes sense, where do you think we upload the production code from? from this same repo! haha the easier it is for me to just upload it to the server once I want to release a version the best, that way I can hook that into our CI/CD pipeline to upload new versions

iLLiCiTiT commented 5 months ago

Of course it makes sense, where do you think we upload the production code from? from this same repo!

The idea is that you should not use the repository to upload the package. You should use packages from releases or ynput cloud.

haha the easier it is for me to just upload it to the server once I want to release a version the best, that way I can hook that into our CI/CD pipeline to upload new versions

So the CI can also upload it, or?

fabiaserra commented 5 months ago

The idea is that you should not use the repository to upload the package. You should use packages from releases or ynput cloud.

Is that assuming that no studio will have their custom fork of the addon? I already have a bunch of stuff on top of the ynput repo?

So the CI can also upload it, or?

Yeah, it would build the package and upload it. I already do that for the ayon-launcher in our internal gitlab CI/CD runners

iLLiCiTiT commented 5 months ago

Is that assuming that no studio will have their custom fork of the addon? I already have a bunch of stuff on top of the ynput repo?

The assumption is that create_package.py is used to create package.

Yeah, it would build the package and upload it. I already do that for the ayon-launcher in our internal gitlab CI/CD runners

So why to add it to create_package.py script? It adds unnecessary complexity to the script, to do something which can be done vie CI/CD?

fabiaserra commented 5 months ago

The assumption is that create_package.py is used to create package.

https://github.com/ynput/ayon-kitsu/blob/develop/create_package.py#L496 :)

So why to add it to create_package.py script? It adds unnecessary complexity to the script, to do something which can be done vie CI/CD?

So other people can benefit from it?

BigRoy commented 5 months ago

So why to add it to create_package.py script? It adds unnecessary complexity to the script, to do something which can be done vie CI/CD?

If running the create_package code from a command line interface makes it easy to detect what zip file(s) it ended up generating without 'guessing' then it wouldn't be too hard to have an extra separate step that is just a simple script to e.g. which would work for ALL create packages:

upload maya.zip houdini.zip core.zip

You could create a temporary directory, then create the packages there with --output tempdir argument added. Then take any of the zips from the output directory (or even allow your custom upload.py script to take the folder path:

create_packages --output tempdir
upload tempdir

As such - I do think it could be fine to have it separate. But I did often have the issue that I directly wanted to upload as well for my dev server as I created a new zip. I've actually never created a zip myself and thought "let's not upload that" except for the AYON addons where I might skip uploading certain addons if I did create multiples in this run.


By the way, wanted to link this community topic since @EmberLightVFX relied on the upload behavior of the kitsu plug-in but combining it with updating the services (which may also show that there's merit in having it as a separate step).

Anyway, definitely wanted to upload quickly usually on zip creations. How it's implemented I don't really care as long as I can do it easily.

The fact that the upload requires access to the server may be good reason to keep it out of the create packages logic - since that by itself, does not need any access? And also avoids e.g. ayon api dependency in all create_package.py files.

fabiaserra commented 5 months ago

Yeah I'm not arguing so much the implementation but the usefulness of having an easy way on a CLI to upload the generated packages to the server. In my mind it makes sense to have it in the same create_package CLI as that's what has the information about the generated zip and in most addons there's a single .zip being generated so I don't need a way to batch upload a bunch of them, but if that's how you'd rather implement it please be it. However, I think @iLLiCiTiT is arguing that there's no point to facilitate the upload

fabiaserra commented 5 months ago

Also, what's the point of having a dev bundle in the production server if it's not actually for deving? Why is it expected that developers need to use a local server to dev?

BigRoy commented 5 months ago

Also, what's the point of having a dev bundle in the production server if it's not actually for deving? Why is it expected that developers need to use a local server to dev?

From my POV:

Well, there is also a backend server that could be updated and tested against with potential database migrations between updates which you may want to test in some cases? Also, I tend to upload addon versions all the time to my dev branch - even addon versions from upstream or other PRs and I don't want to care at all whether I might be installing on top of an existing addon version on our production server.

So I'd say, actual testing on a development server helps for a lot of extra safety/security. I still use dev bundle on the production server just to test whether my local runs do anything odd on the production server once I know it's 'somewhat stable'. I use that dev mode to check/debug/fix issues specific to a particular workfile quickly - by using the dev mode running with local packages.

fabiaserra commented 5 months ago

Oh for sure, I'm not neglecting the benefit of being able to run a local container, it has so many benefits! What I'm saying is that the production server with the dev bundle also has this benefit of allowing to test dev iterations, which proves the need for being able to do this easily even for deving!

BigRoy commented 5 months ago

Oh for sure, I'm not neglecting the benefit of being able to run a local container, it has so many benefits! What I'm saying is that the production server with the dev bundle also has this benefit of allowing to test dev iterations, which proves the need for being able to do this easily even for deving!

I think the biggest danger there is that if you run this often you'd also be taking down the server often because it currently requires a restart.

fabiaserra commented 5 months ago

I think the biggest danger there is that if you run this often you'd also be taking down the server often because it currently requires a restart.

Fair enough yeah, I guess since we are still quite far from moving to AYON and I'm the only one testing the "production" server makes this argument different for me, I will probably change the way I work once we do move to AYON

fabiaserra commented 5 months ago

I ended up adding it on my fork by just copy pasting ayon-kitsu's code, if anyone is interested, here's the diff:

diff --git a/create_package.py b/create_package.py
index 442c191..10ea720 100644
--- a/create_package.py
+++ b/create_package.py
@@ -38,6 +38,22 @@ import zipfile
 from typing import Optional
 import package

+try:
+    import ayon_api
+    from ayon_api import get_server_api_connection
+
+    has_ayon_api = True
+except ModuleNotFoundError:
+    has_ayon_api = False
+
+try:
+    from dotenv import load_dotenv
+
+    load_dotenv()
+except ModuleNotFoundError:
+    if has_ayon_api:
+        logging.warning("dotenv not installed, skipping loading .env file")
+
 ADDON_NAME: str = package.name
 ADDON_VERSION: str = package.version
 ADDON_CLIENT_DIR: str = package.client_dir
@@ -254,7 +270,7 @@ def main(
     output_dir: Optional[str] = None,
     skip_zip: bool = False,
     keep_sources: bool = False,
-    clear_output_dir: bool = False
+    clear_output_dir: bool = False,
 ):
     log = logging.getLogger("create_package")
     log.info("Start creating package")
@@ -334,11 +350,40 @@ if __name__ == "__main__":
             " (Will be purged if already exists!)"
         )
     )
+    parser.add_argument(
+        "--upload",
+        dest="upload",
+        action="store_true",
+        help="Upload the build to your ayon server and reload",
+    )

     args = parser.parse_args(sys.argv[1:])
     main(
         args.output_dir,
         args.skip_zip,
         args.keep_sources,
-        args.clear_output_dir
+        args.clear_output_dir,
     )
+    if args.upload and not args.skip_zip:
+        if not has_ayon_api:
+            raise RuntimeError(
+                "Ayon API is not available. Please install it"
+                " to use the upload feature."
+            )
+        output_dir = args.output_dir
+        current_dir = os.path.dirname(os.path.abspath(__file__))
+        if not output_dir:
+            output_dir = os.path.join(current_dir, "package")
+        output_path = os.path.join(
+            output_dir, f"{ADDON_NAME}-{ADDON_VERSION}.zip"
+        )
+
+        ayon_api.init_service()
+        log: logging.Logger = logging.getLogger("upload_package")
+        log.info("Trying to upload zip")
+        response = ayon_api.upload_addon_zip(output_path)
+        server = get_server_api_connection()
+        if server:
+            server.trigger_server_restart()
+        else:
+            log.warning("Could not restart server")
\ No newline at end of file