Pass a callback instead of weak linking a module

Signed-off-by: AMZN-Phil <pconroy@amazon.com>

Code/Tools/ProjectManager/Source/PythonBindings.cpp

@@ -223,49 +223,6 @@ namespace RedirectOutput
     }
 } // namespace RedirectOutput
 
-namespace O3DEProjectManagerPy
-{
-    using DownloadProgressFunc = AZStd::function<void(int)>;
-    DownloadProgressFunc currentProgressCallback;
-    bool requestCancelDownload = false;
-
-    static PyObject* CLICancelDownload(PyObject* /*self*/, PyObject* /*args*/)
-    {
-        if (requestCancelDownload)
-        {
-            return Py_True;
-        }
-        else
-        {
-            return Py_False;
-        }
-    }
-
-    static PyObject* CLIDownloadProgress(PyObject* /*self*/, PyObject* args)
-    {
-        int progress_percentage = 0;
-        if (!PyArg_ParseTuple(args, "i", &progress_percentage))
-            return NULL;
-        if (currentProgressCallback)
-        {
-            currentProgressCallback(progress_percentage);
-        }
-
-        Py_RETURN_NONE;
-    }
-
-    static PyMethodDef O3DEPMPyMethods[] = { { "download_progress", CLIDownloadProgress, METH_VARARGS, "Used to call back to the UI to inform of download progress." },
-                                             { "request_cancel_download", CLICancelDownload, METH_NOARGS, "Returns that the UI is requesting that the current download be cancelled." },
-                                             { NULL, NULL, 0, NULL } };
-
-    static PyModuleDef O3DEPMPyModule = { PyModuleDef_HEAD_INIT, "o3de_projectmanager", NULL, -1, O3DEPMPyMethods, NULL, NULL, NULL, NULL };
-
-    static PyObject* PyInit_O3DEPMPy(void)
-    {
-        return PyModule_Create(&O3DEPMPyModule);
-    }
-} // namespace O3DEProjectManagerPy
-
 
 namespace O3DE::ProjectManager
 {
@@ -314,7 +271,6 @@ namespace O3DE::ProjectManager
         AZ_TracePrintf("python", "Py_GetProgramFullPath=%ls \n", Py_GetProgramFullPath());
 
         PyImport_AppendInittab("azlmbr_redirect", RedirectOutput::PyInit_RedirectOutput);
-        PyImport_AppendInittab("o3de_projectmanager", &O3DEProjectManagerPy::PyInit_O3DEPMPy);
 
         try
         {
@@ -1126,20 +1082,26 @@ namespace O3DE::ProjectManager
     {
         // This process is currently limited to download a single gem at a time.
         bool downloadSucceeded = false;
-        O3DEProjectManagerPy::currentProgressCallback = gemProgressCallback;
+
-        O3DEProjectManagerPy::requestCancelDownload = false;
+        m_requestCancelDownload = false;
         auto result = ExecuteWithLockErrorHandling(
             [&]
             {
                 auto downloadResult = m_download.attr("download_gem")(
                     QString_To_Py_String(gemName), // gem name
                     pybind11::none(), // destination path
-                    false// skip auto register
+                    false, // skip auto register
+                    pybind11::cpp_function(
+                        [this, gemProgressCallback](int progress)
+                        {
+                            gemProgressCallback(progress);
+
+                            return m_requestCancelDownload;
+                        }) // Callback for download progress and cancelling
                     );
                 downloadSucceeded = (downloadResult.cast<int>() == 0);
             });
 
-        O3DEProjectManagerPy::currentProgressCallback = nullptr;
 
         if (!result.IsSuccess())
         {
@@ -1155,6 +1117,6 @@ namespace O3DE::ProjectManager
 
     void PythonBindings::CancelDownload()
     {
-        O3DEProjectManagerPy::requestCancelDownload = true;
+        m_requestCancelDownload = true;
     }
 }

Code/Tools/ProjectManager/Source/PythonBindings.h

@@ -91,5 +91,7 @@ namespace O3DE::ProjectManager
         pybind11::handle m_editProjectProperties;
         pybind11::handle m_download;
         pybind11::handle m_pathlib;
+
+        bool m_requestCancelDownload = false;
     };
 }

scripts/o3de/o3de/download.py

@@ -90,7 +90,8 @@ def get_downloadable(engine_name: str = None,
 
 
 def download_o3de_object(object_name: str, default_folder_name: str, dest_path: str or pathlib.Path,
-                         object_type: str, downloadable_kwarg_key, skip_auto_register: bool) -> int:
+                         object_type: str, downloadable_kwarg_key, skip_auto_register: bool,
+                         download_progress_callback = None) -> int:
 
     download_path = manifest.get_o3de_cache_folder() / default_folder_name / object_name
     download_path.mkdir(parents=True, exist_ok=True)
@@ -104,7 +105,7 @@ def download_o3de_object(object_name: str, default_folder_name: str, dest_path:
     origin_uri = downloadable_object_data['originuri']
     parsed_uri = urllib.parse.urlparse(origin_uri)
 
-    download_zip_result = utils.download_zip_file(parsed_uri, download_zip_path)
+    download_zip_result = utils.download_zip_file(parsed_uri, download_zip_path, download_progress_callback)
     if download_zip_result != 0:
         return download_zip_result
 
@@ -147,33 +148,38 @@ def download_o3de_object(object_name: str, default_folder_name: str, dest_path:
 
 def download_engine(engine_name: str,
                     dest_path: str or pathlib.Path,
-                    skip_auto_register: bool) -> int:
+                    skip_auto_register: bool,
-    return download_o3de_object(engine_name, 'engines', dest_path, 'engine', 'engine_name', skip_auto_register)
+                    download_progress_callback = None) -> int:
+    return download_o3de_object(engine_name, 'engines', dest_path, 'engine', 'engine_name', skip_auto_register, download_progress_callback)
 
 
 def download_project(project_name: str,
                      dest_path: str or pathlib.Path,
-                     skip_auto_register: bool) -> int:
+                     skip_auto_register: bool,
-    return download_o3de_object(project_name, 'projects', dest_path, 'project', 'project_name', skip_auto_register)
+                     download_progress_callback = None) -> int:
+    return download_o3de_object(project_name, 'projects', dest_path, 'project', 'project_name', skip_auto_register, download_progress_callback)
 
 
 def download_gem(gem_name: str,
                  dest_path: str or pathlib.Path,
-                 skip_auto_register: bool) -> int:
+                 skip_auto_register: bool,
-    return download_o3de_object(gem_name, 'gems', dest_path, 'gem', 'gem_name', skip_auto_register)
+                 download_progress_callback = None) -> int:
+    return download_o3de_object(gem_name, 'gems', dest_path, 'gem', 'gem_name', skip_auto_register, download_progress_callback)
 
 
 def download_template(template_name: str,
                       dest_path: str or pathlib.Path,
-                      skip_auto_register: bool) -> int:
+                      skip_auto_register: bool,
-    return download_o3de_object(template_name, 'templates', dest_path, 'template', 'template_name', skip_auto_register)
+                      download_progress_callback = None) -> int:
+    return download_o3de_object(template_name, 'templates', dest_path, 'template', 'template_name', skip_auto_register, download_progress_callback)
 
 
 
 def download_restricted(restricted_name: str,
                         dest_path: str or pathlib.Path,
-                        skip_auto_register: bool) -> int:
+                        skip_auto_register: bool,
-    return download_o3de_object(restricted_name, 'restricted', dest_path, 'restricted', 'restricted_name', skip_auto_register)
+                        download_progress_callback = None) -> int:
+    return download_o3de_object(restricted_name, 'restricted', dest_path, 'restricted', 'restricted_name', skip_auto_register, download_progress_callback)
 
 
 def _run_download(args: argparse) -> int:

scripts/o3de/o3de/utils.py

@@ -16,10 +16,6 @@ import shutil
 import urllib.request
 import logging
 import zipfile
-try:
-    import o3de_projectmanager
-except ImportError:
-    pass
 
 logger = logging.getLogger()
 logging.basicConfig()
@@ -38,14 +34,13 @@ def copyfileobj(fsrc, fdst, callback, length=0):
 
     copied = 0
     while True:
-        if o3de_projectmanager and o3de_projectmanager.request_cancel_download():
-            return 1
         buf = fsrc_read(length)
         if not buf:
             break
         fdst_write(buf)
         copied += len(buf)
-        callback(copied)
+        if callback(copied):
+            return 1
     return 0
 
 def validate_identifier(identifier: str) -> bool:
@@ -122,11 +117,12 @@ def backup_folder(folder: str or pathlib.Path) -> None:
             if backup_folder_name.is_dir():
                 renamed = True
 
-def download_file(parsed_uri, download_path: pathlib.Path) -> int:
+def download_file(parsed_uri, download_path: pathlib.Path, download_progress_callback = None) -> int:
     """
     :param parsed_uri: uniform resource identifier to zip file to download
     :param download_path: location path on disk to download file
     """
+    logger.warn(f'File about to downloaded to {download_path}.')
     if download_path.is_file():
         logger.warn(f'File already downloaded to {download_path}.')
     elif parsed_uri.scheme in ['http', 'https', 'ftp', 'ftps']:
@@ -137,8 +133,8 @@ def download_file(parsed_uri, download_path: pathlib.Path) -> int:
             except KeyError:
                 pass
             def download_progress(blocks):
-                if o3de_projectmanager and download_file_size:
+                if download_progress_callback and download_file_size:
-                    o3de_projectmanager.download_progress(int(blocks/int(download_file_size) * 100))
+                    return download_progress_callback(int(blocks/int(download_file_size) * 100))
             with download_path.open('wb') as f:
                 download_cancelled = copyfileobj(s, f, download_progress)
                 if download_cancelled:
@@ -152,12 +148,12 @@ def download_file(parsed_uri, download_path: pathlib.Path) -> int:
     return 0
 
 
-def download_zip_file(parsed_uri, download_zip_path: pathlib.Path) -> int:
+def download_zip_file(parsed_uri, download_zip_path: pathlib.Path, download_progress_callback = None) -> int:
     """
     :param parsed_uri: uniform resource identifier to zip file to download
     :param download_zip_path: path to output zip file
     """
-    download_file_result = download_file(parsed_uri, download_zip_path)
+    download_file_result = download_file(parsed_uri, download_zip_path, download_progress_callback)
     if download_file_result != 0:
         return download_file_result