From 118834efdedb4ebe5290c2de1826ef2e562a5741 Mon Sep 17 00:00:00 2001 From: nggieber Date: Mon, 4 Oct 2021 15:08:13 -0700 Subject: [PATCH 1/5] Added ability to list gem repos using CLI and integrated support into Project Manager Signed-off-by: nggieber --- .../Source/GemRepo/GemRepoInfo.h | 2 +- .../Source/GemRepo/GemRepoItemDelegate.cpp | 3 +- .../Source/GemRepo/GemRepoScreen.cpp | 21 ++ .../Source/ProjectManagerDefs.h | 2 + .../ProjectManager/Source/PythonBindings.cpp | 60 +++++- .../ProjectManager/Source/PythonBindings.h | 2 +- scripts/o3de/o3de/manifest.py | 190 +++++++----------- scripts/o3de/o3de/validation.py | 1 - 8 files changed, 152 insertions(+), 129 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h index 14c76bd0c2..61220cefe7 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoInfo.h @@ -30,7 +30,7 @@ namespace O3DE::ProjectManager bool operator<(const GemRepoInfo& gemRepoInfo) const; QString m_path = ""; - QString m_name = "Unknown Gem Repo Name"; + QString m_name = "Unknown Repo Name"; QString m_creator = "Unknown Creator"; bool m_isEnabled = false; //! Is the repo currently enabled for this engine? QString m_summary = "No summary provided."; diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoItemDelegate.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoItemDelegate.cpp index 88ccee2636..58b10a1d1a 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoItemDelegate.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoItemDelegate.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -95,7 +96,7 @@ namespace O3DE::ProjectManager painter->drawText(repoCreatorRect, Qt::TextSingleLine, repoCreator); // Repo update - QString repoUpdatedDate = GemRepoModel::GetLastUpdated(modelIndex).toString("dd/MM/yyyy hh:mmap"); + QString repoUpdatedDate = GemRepoModel::GetLastUpdated(modelIndex).toString(RepoTimeFormat); repoUpdatedDate = standardFontMetrics.elidedText(repoUpdatedDate, Qt::TextElideMode::ElideRight, s_updatedMaxWidth); QRect repoUpdatedDateRect = GetTextRect(standardFont, repoUpdatedDate, s_fontSize); diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp index 9c432884e6..a233010225 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -104,9 +105,29 @@ namespace O3DE::ProjectManager { // Add all available repos to the model const QVector allGemRepoInfos = allGemRepoInfosResult.GetValue(); + QDateTime oldestRepoUpdate; + if (!allGemRepoInfos.isEmpty()) + { + oldestRepoUpdate = allGemRepoInfos[0].m_lastUpdated; + } for (const GemRepoInfo& gemRepoInfo : allGemRepoInfos) { m_gemRepoModel->AddGemRepo(gemRepoInfo); + + // Find least recently updated repo + if (gemRepoInfo.m_lastUpdated < oldestRepoUpdate) + { + oldestRepoUpdate = gemRepoInfo.m_lastUpdated; + } + } + + if (!allGemRepoInfos.isEmpty()) + { + m_lastAllUpdateLabel->setText(tr("Last Updated: %1").arg(oldestRepoUpdate.toString(RepoTimeFormat))); + } + else + { + m_lastAllUpdateLabel->setText(tr("Last Updated: Never")); } } else diff --git a/Code/Tools/ProjectManager/Source/ProjectManagerDefs.h b/Code/Tools/ProjectManager/Source/ProjectManagerDefs.h index 264515652f..e8e290ad02 100644 --- a/Code/Tools/ProjectManager/Source/ProjectManagerDefs.h +++ b/Code/Tools/ProjectManager/Source/ProjectManagerDefs.h @@ -26,4 +26,6 @@ namespace O3DE::ProjectManager static const QString ProjectCMakeCommand = "cmake"; static const QString ProjectCMakeBuildTargetEditor = "Editor"; + static const QString RepoTimeFormat = "dd/MM/yyyy hh:mmap"; + } // namespace O3DE::ProjectManager diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index b9369b5bb0..96c54d8af2 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -929,13 +929,55 @@ namespace O3DE::ProjectManager return AZ::Failure("Adding Gem Repo not implemented yet in o3de scripts."); } - GemRepoInfo PythonBindings::GemRepoInfoFromPath(pybind11::handle path, pybind11::handle pyEnginePath) + GemRepoInfo PythonBindings::GetGemRepoInfo(pybind11::handle repoUri) { - /* Placeholder Logic */ - (void)path; - (void)pyEnginePath; + GemRepoInfo gemRepoInfo; + gemRepoInfo.m_repoLink = Py_To_String(repoUri); - return GemRepoInfo(); + auto data = m_manifest.attr("get_repo_json_data")(repoUri); + if (pybind11::isinstance(data)) + { + try + { + // required + gemRepoInfo.m_repoLink = Py_To_String(data["repo_uri"]); + gemRepoInfo.m_name = Py_To_String(data["repo_name"]); + gemRepoInfo.m_creator = Py_To_String(data["origin"]); + + // optional + gemRepoInfo.m_summary = Py_To_String_Optional(data, "summary", "No summary provided."); + gemRepoInfo.m_additionalInfo = Py_To_String_Optional(data, "additional_info", ""); + + auto repoPath = m_manifest.attr("get_repo_path")(repoUri); + gemRepoInfo.m_path = gemRepoInfo.m_directoryLink = Py_To_String(repoPath); + + QString lastUpdated = Py_To_String_Optional(data, "last_updated", ""); + gemRepoInfo.m_lastUpdated = QDateTime::fromString(lastUpdated, RepoTimeFormat); + + if (data.contains("enabled")) + { + gemRepoInfo.m_isEnabled = data["enabled"].cast(); + } + else + { + gemRepoInfo.m_isEnabled = false; + } + + if (data.contains("gem_paths")) + { + for (auto gemPath : data["gem_paths"]) + { + gemRepoInfo.m_includedGemPaths.push_back(Py_To_String(gemPath)); + } + } + } + catch ([[maybe_unused]] const std::exception& e) + { + AZ_Warning("PythonBindings", false, "Failed to get GemRepoInfo for repo %s", Py_To_String(repoUri)); + } + } + + return gemRepoInfo; } //#define MOCK_GEM_REPO_INFO true @@ -948,14 +990,10 @@ namespace O3DE::ProjectManager auto result = ExecuteWithLockErrorHandling( [&] { - /* Placeholder Logic, o3de scripts need method added - * - for (auto path : m_manifest.attr("get_gem_repos")()) + for (auto repoUri : m_manifest.attr("get_repos")()) { - gemRepos.push_back(GemRepoInfoFromPath(path, pybind11::none())); + gemRepos.push_back(GetGemRepoInfo(repoUri)); } - * - */ }); if (!result.IsSuccess()) { diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.h b/Code/Tools/ProjectManager/Source/PythonBindings.h index 42f04ed6e6..195915a18f 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.h +++ b/Code/Tools/ProjectManager/Source/PythonBindings.h @@ -66,7 +66,7 @@ namespace O3DE::ProjectManager AZ::Outcome ExecuteWithLockErrorHandling(AZStd::function executionCallback); bool ExecuteWithLock(AZStd::function executionCallback); GemInfo GemInfoFromPath(pybind11::handle path, pybind11::handle pyProjectPath); - GemRepoInfo GemRepoInfoFromPath(pybind11::handle path, pybind11::handle pyEnginePath); + GemRepoInfo GetGemRepoInfo(pybind11::handle repoUri); ProjectInfo ProjectInfoFromPath(pybind11::handle path); ProjectTemplateInfo ProjectTemplateInfoFromPath(pybind11::handle path, pybind11::handle pyProjectPath); bool RegisterThisEngine(); diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index 7e504d29cd..ccb5cda761 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -13,8 +13,10 @@ import json import logging import os import pathlib +import shutil +import hashlib -from o3de import validation +from o3de import validation, utils logger = logging.getLogger() logging.basicConfig() @@ -135,12 +137,12 @@ def get_o3de_manifest() -> pathlib.Path: json_data.update({'default_restricted_folder': default_restricted_folder.as_posix()}) json_data.update({'default_third_party_folder': default_third_party_folder.as_posix()}) + json_data.update({'engines': []}) json_data.update({'projects': []}) json_data.update({'external_subdirectories': []}) json_data.update({'templates': []}) json_data.update({'restricted': []}) json_data.update({'repos': []}) - json_data.update({'engines': []}) default_restricted_folder_json = default_restricted_folder / 'restricted.json' if not default_restricted_folder_json.is_file(): @@ -195,23 +197,25 @@ def load_o3de_manifest(manifest_path: pathlib.Path = None) -> dict: return json_data -def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> bool: +def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> None: """ - Save the json dictionary to the supplied manifest file or ~/.o3de/o3de_manifest.json if manifest_path is None + Save the json dictionary to the supplied manifest file or ~/.o3de/o3de_manifest.json if None - :param json_data: dictionary to save in json format at the file path - :param manifest_path: optional path to manifest file to save - """ + :param json_data: dictionary to save in json format at the file path + :param manifest_path: optional path to manifest file to save + """ if not manifest_path: manifest_path = get_o3de_manifest() + backup_name = utils.backup_file(manifest_path) with manifest_path.open('w') as s: try: s.write(json.dumps(json_data, indent=4) + '\n') - return True - except OSError as e: + except Exception as e: logger.error(f'Manifest json failed to save: {str(e)}') - return False - + os.unlink(manifest_path) + os.rename(backup_name, manifest_path) + finally: + os.unlink(backup_name) def get_gems_from_subdirectories(external_subdirs: list) -> list: @@ -236,6 +240,12 @@ def get_gems_from_subdirectories(external_subdirs: list) -> list: # Data query methods +def get_this_engine() -> dict: + json_data = load_o3de_manifest() + engine_data = find_engine_data(json_data) + return engine_data + + def get_engines() -> list: json_data = load_o3de_manifest() engine_list = json_data['engines'] if 'engines' in json_data else [] @@ -421,40 +431,48 @@ def get_templates_for_generic_creation(): # temporary until we have a better wa return list(filter(filter_project_and_gem_templates_out, get_all_templates())) - -def get_engine_json_data(engine_name: str = None, - engine_path: str or pathlib.Path = None) -> dict or None: - if not engine_name and not engine_path: - logger.error('Must specify either a Engine name or Engine Path.') +def get_json_data(object_typename: str = None, + object_path: str or pathlib.Path = None, + object_validator = callable) -> dict or None: + if not object_typename or not object_validator: + logger.error(f'Missing object info.') + + if not object_path: + logger.error(f'{object_typename} Path {object_path} has not been registered.') return None - if engine_name and not engine_path: - engine_path = get_registered(engine_name=engine_name) - - if not engine_path: - logger.error(f'Engine Path {engine_path} has not been registered.') + object_path = pathlib.Path(object_path).resolve() + object_json = object_path / f'{object_typename}.json' + if not object_json.is_file(): + logger.error(f'{object_typename} json {object_json} is not present.') return None - - engine_path = pathlib.Path(engine_path).resolve() - engine_json = engine_path / 'engine.json' - if not engine_json.is_file(): - logger.error(f'Engine json {engine_json} is not present.') - return None - if not validation.valid_o3de_engine_json(engine_json): - logger.error(f'Engine json {engine_json} is not valid.') + if not object_validator(object_json): + logger.error(f'{object_typename} json {object_json} is not valid.') return None - with engine_json.open('r') as f: + with object_json.open('r') as f: try: - engine_json_data = json.load(f) + object_json_data = json.load(f) except json.JSONDecodeError as e: - logger.warn(f'{engine_json} failed to load: {str(e)}') + logger.warn(f'{object_json} failed to load: {str(e)}') else: - return engine_json_data + return object_json_data return None +def get_engine_json_data(engine_name: str = None, + engine_path: str or pathlib.Path = None) -> dict or None: + if not engine_name and not engine_path: + logger.error('Must specify either a Engine name or Engine Path.') + return None + + if engine_name and not engine_path: + engine_path = get_registered(engine_name=engine_name) + + return get_json_data('engine', engine_path, validation.valid_o3de_engine_json) + + def get_project_json_data(project_name: str = None, project_path: str or pathlib.Path = None) -> dict or None: if not project_name and not project_path: @@ -464,28 +482,7 @@ def get_project_json_data(project_name: str = None, if project_name and not project_path: project_path = get_registered(project_name=project_name) - if not project_path: - logger.error(f'Project Path {project_path} has not been registered.') - return None - - project_path = pathlib.Path(project_path).resolve() - project_json = project_path / 'project.json' - if not project_json.is_file(): - logger.error(f'Project json {project_json} is not present.') - return None - if not validation.valid_o3de_project_json(project_json): - logger.error(f'Project json {project_json} is not valid.') - return None - - with project_json.open('r') as f: - try: - project_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warn(f'{project_json} failed to load: {str(e)}') - else: - return project_json_data - - return None + return get_json_data('project', project_path, validation.valid_o3de_project_json) def get_gem_json_data(gem_name: str = None, gem_path: str or pathlib.Path = None, @@ -497,28 +494,7 @@ def get_gem_json_data(gem_name: str = None, gem_path: str or pathlib.Path = None if gem_name and not gem_path: gem_path = get_registered(gem_name=gem_name, project_path=project_path) - if not gem_path: - logger.error(f'Gem Path {gem_path} has not been registered.') - return None - - gem_path = pathlib.Path(gem_path).resolve() - gem_json = gem_path / 'gem.json' - if not gem_json.is_file(): - logger.error(f'Gem json {gem_json} is not present.') - return None - if not validation.valid_o3de_gem_json(gem_json): - logger.error(f'Gem json {gem_json} is not valid.') - return None - - with gem_json.open('r') as f: - try: - gem_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warn(f'{gem_json} failed to load: {str(e)}') - else: - return gem_json_data - - return None + return get_json_data('gem', gem_path, validation.valid_o3de_gem_json) def get_template_json_data(template_name: str = None, template_path: str or pathlib.Path = None, @@ -530,28 +506,7 @@ def get_template_json_data(template_name: str = None, template_path: str or path if template_name and not template_path: template_path = get_registered(template_name=template_name, project_path=project_path) - if not template_path: - logger.error(f'Template Path {template_path} has not been registered.') - return None - - template_path = pathlib.Path(template_path).resolve() - template_json = template_path / 'template.json' - if not template_json.is_file(): - logger.error(f'Template json {template_json} is not present.') - return None - if not validation.valid_o3de_template_json(template_json): - logger.error(f'Template json {template_json} is not valid.') - return None - - with template_json.open('r') as f: - try: - template_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warn(f'{template_json} failed to load: {str(e)}') - else: - return template_json_data - - return None + return get_json_data('template', template_path, validation.valid_o3de_template_json) def get_restricted_json_data(restricted_name: str = None, restricted_path: str or pathlib.Path = None, @@ -563,29 +518,38 @@ def get_restricted_json_data(restricted_name: str = None, restricted_path: str o if restricted_name and not restricted_path: restricted_path = get_registered(restricted_name=restricted_name, project_path=project_path) - if not restricted_path: - logger.error(f'Restricted Path {restricted_path} has not been registered.') + return get_json_data('restricted', restricted_path, validation.valid_o3de_restricted_json) + +def get_repo_json_data(repo_uri: str = None) -> dict or None: + if not repo_uri: + logger.error('Must specify a Repo Uri.') return None - restricted_path = pathlib.Path(restricted_path).resolve() - restricted_json = restricted_path / 'restricted.json' - if not restricted_json.is_file(): - logger.error(f'Restricted json {restricted_json} is not present.') + repo_json = get_repo_path(repo_uri=repo_uri) + + if not repo_json.is_file(): + logger.error(f'Repo json {repo_json} is not present.') return None - if not validation.valid_o3de_restricted_json(restricted_json): - logger.error(f'Restricted json {restricted_json} is not valid.') + if not validation.valid_o3de_repo_json(repo_json): + logger.error(f'Repo json {repo_json} is not valid.') return None - with restricted_json.open('r') as f: + with repo_json.open('r') as f: try: - restricted_json_data = json.load(f) + repo_json_data = json.load(f) except json.JSONDecodeError as e: - logger.warn(f'{restricted_json} failed to load: {str(e)}') + logger.warn(f'{repo_json} failed to load: {str(e)}') else: - return restricted_json_data + return repo_json_data return None +def get_repo_path(repo_uri: str = None, cache_folder: str = None) -> pathlib.Path: + if not cache_folder: + cache_folder = get_o3de_cache_folder() + + repo_sha256 = hashlib.sha256(repo_uri.encode()) + return cache_folder / str(repo_sha256.hexdigest() + '.json') def get_registered(engine_name: str = None, project_name: str = None, @@ -721,9 +685,7 @@ def get_registered(engine_name: str = None, elif isinstance(repo_name, str): cache_folder = get_o3de_cache_folder() for repo_uri in json_data['repos']: - repo_uri = pathlib.Path(repo_uri).resolve() - repo_sha256 = hashlib.sha256(repo_uri.encode()) - cache_file = cache_folder / str(repo_sha256.hexdigest() + '.json') + cache_file = get_repo_path(repo_uri=repo_uri, cache_folder=cache_folder) if cache_file.is_file(): repo = pathlib.Path(cache_file).resolve() with repo.open('r') as f: diff --git a/scripts/o3de/o3de/validation.py b/scripts/o3de/o3de/validation.py index 8fcc0d7e7d..5c683f0667 100644 --- a/scripts/o3de/o3de/validation.py +++ b/scripts/o3de/o3de/validation.py @@ -27,7 +27,6 @@ def valid_o3de_repo_json(file_name: str or pathlib.Path) -> bool: test = json_data['origin'] except (json.JSONDecodeError, KeyError) as e: return False - return True From acfbed8a58d37512ef75c1098ed807cd8699444c Mon Sep 17 00:00:00 2001 From: nggieber Date: Wed, 6 Oct 2021 06:32:28 -0700 Subject: [PATCH 2/5] Fix minor PR feedback Signed-off-by: nggieber --- scripts/o3de/o3de/manifest.py | 67 ++++++++++++++++------------------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index ccb5cda761..32bab05de8 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -197,7 +197,7 @@ def load_o3de_manifest(manifest_path: pathlib.Path = None) -> dict: return json_data -def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> None: +def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> bool: """ Save the json dictionary to the supplied manifest file or ~/.o3de/o3de_manifest.json if None @@ -206,16 +206,13 @@ def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> N """ if not manifest_path: manifest_path = get_o3de_manifest() - backup_name = utils.backup_file(manifest_path) with manifest_path.open('w') as s: try: s.write(json.dumps(json_data, indent=4) + '\n') - except Exception as e: + return True + except OSError as e: logger.error(f'Manifest json failed to save: {str(e)}') - os.unlink(manifest_path) - os.rename(backup_name, manifest_path) - finally: - os.unlink(backup_name) + return False def get_gems_from_subdirectories(external_subdirs: list) -> list: @@ -239,13 +236,6 @@ def get_gems_from_subdirectories(external_subdirs: list) -> list: return gem_directories -# Data query methods -def get_this_engine() -> dict: - json_data = load_o3de_manifest() - engine_data = find_engine_data(json_data) - return engine_data - - def get_engines() -> list: json_data = load_o3de_manifest() engine_list = json_data['engines'] if 'engines' in json_data else [] @@ -431,21 +421,32 @@ def get_templates_for_generic_creation(): # temporary until we have a better wa return list(filter(filter_project_and_gem_templates_out, get_all_templates())) -def get_json_data(object_typename: str = None, - object_path: str or pathlib.Path = None, - object_validator = callable) -> dict or None: - if not object_typename or not object_validator: - logger.error(f'Missing object info.') +def get_json_file_path(object_typename: str = None, + object_path: str or pathlib.Path = None) -> pathlib.Path: + if not object_typename: + logger.error(f'Missing object typename.') if not object_path: logger.error(f'{object_typename} Path {object_path} has not been registered.') return None object_path = pathlib.Path(object_path).resolve() - object_json = object_path / f'{object_typename}.json' + return object_path / f'{object_typename}.json' + + +def get_json_data_file(object_typename: str = None, + object_json: str or pathlib.Path = None, + object_validator = callable) -> dict or None: + if not object_typename: + logger.error(f'Missing object typename.') + if not object_json.is_file(): logger.error(f'{object_typename} json {object_json} is not present.') return None + + if not object_validator: + logger.error(f'Missing object validator.') + if not object_validator(object_json): logger.error(f'{object_typename} json {object_json} is not valid.') return None @@ -454,12 +455,19 @@ def get_json_data(object_typename: str = None, try: object_json_data = json.load(f) except json.JSONDecodeError as e: - logger.warn(f'{object_json} failed to load: {str(e)}') + logger.warn(f'{object_json} failed to load: {e}') else: return object_json_data return None +def get_json_data(object_typename: str = None, + object_path: str or pathlib.Path = None, + object_validator = callable) -> dict or None: + object_json = get_json_file_path(object_typename, object_path) + + return get_json_data_file(object_typename, object_json, object_validator) + def get_engine_json_data(engine_name: str = None, engine_path: str or pathlib.Path = None) -> dict or None: @@ -527,22 +535,7 @@ def get_repo_json_data(repo_uri: str = None) -> dict or None: repo_json = get_repo_path(repo_uri=repo_uri) - if not repo_json.is_file(): - logger.error(f'Repo json {repo_json} is not present.') - return None - if not validation.valid_o3de_repo_json(repo_json): - logger.error(f'Repo json {repo_json} is not valid.') - return None - - with repo_json.open('r') as f: - try: - repo_json_data = json.load(f) - except json.JSONDecodeError as e: - logger.warn(f'{repo_json} failed to load: {str(e)}') - else: - return repo_json_data - - return None + get_json_data_file("Repo", repo_json, validation.valid_o3de_repo_json) def get_repo_path(repo_uri: str = None, cache_folder: str = None) -> pathlib.Path: if not cache_folder: From 06953bb81f57ade284f1bc4daaa03ab93c8b9b7c Mon Sep 17 00:00:00 2001 From: nggieber Date: Fri, 8 Oct 2021 08:14:28 -0700 Subject: [PATCH 3/5] More PR changes Signed-off-by: nggieber --- scripts/o3de/o3de/manifest.py | 41 ++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index 32bab05de8..5d34c4bcf9 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -421,31 +421,34 @@ def get_templates_for_generic_creation(): # temporary until we have a better wa return list(filter(filter_project_and_gem_templates_out, get_all_templates())) -def get_json_file_path(object_typename: str = None, +def get_json_file_path(object_typename: str, object_path: str or pathlib.Path = None) -> pathlib.Path: if not object_typename: - logger.error(f'Missing object typename.') + logger.error('Missing object typename.') + return None if not object_path: - logger.error(f'{object_typename} Path {object_path} has not been registered.') return None object_path = pathlib.Path(object_path).resolve() return object_path / f'{object_typename}.json' -def get_json_data_file(object_typename: str = None, - object_json: str or pathlib.Path = None, +def get_json_data_file(object_json: pathlib.Path, + object_typename: str = None, object_validator = callable) -> dict or None: if not object_typename: - logger.error(f'Missing object typename.') + logger.error('Missing object typename.') + + if not object_json: + logger.error(f'No object_json provided for {object_typename}') if not object_json.is_file(): logger.error(f'{object_typename} json {object_json} is not present.') return None if not object_validator: - logger.error(f'Missing object validator.') + logger.error('Missing object validator.') if not object_validator(object_json): logger.error(f'{object_typename} json {object_json} is not valid.') @@ -463,10 +466,14 @@ def get_json_data_file(object_typename: str = None, def get_json_data(object_typename: str = None, object_path: str or pathlib.Path = None, - object_validator = callable) -> dict or None: + object_validator = callable, + object_name: str = None) -> dict or None: object_json = get_json_file_path(object_typename, object_path) - return get_json_data_file(object_typename, object_json, object_validator) + if not object_json and object_name: + logger.error(f'{object_name} has not been registered.') + + return get_json_data_file(object_json, object_typename, object_validator) def get_engine_json_data(engine_name: str = None, @@ -478,7 +485,7 @@ def get_engine_json_data(engine_name: str = None, if engine_name and not engine_path: engine_path = get_registered(engine_name=engine_name) - return get_json_data('engine', engine_path, validation.valid_o3de_engine_json) + return get_json_data('engine', engine_path, validation.valid_o3de_engine_json, engine_name) def get_project_json_data(project_name: str = None, @@ -490,7 +497,7 @@ def get_project_json_data(project_name: str = None, if project_name and not project_path: project_path = get_registered(project_name=project_name) - return get_json_data('project', project_path, validation.valid_o3de_project_json) + return get_json_data('project', project_path, validation.valid_o3de_project_json, project_name) def get_gem_json_data(gem_name: str = None, gem_path: str or pathlib.Path = None, @@ -502,7 +509,7 @@ def get_gem_json_data(gem_name: str = None, gem_path: str or pathlib.Path = None if gem_name and not gem_path: gem_path = get_registered(gem_name=gem_name, project_path=project_path) - return get_json_data('gem', gem_path, validation.valid_o3de_gem_json) + return get_json_data('gem', gem_path, validation.valid_o3de_gem_json, gem_name) def get_template_json_data(template_name: str = None, template_path: str or pathlib.Path = None, @@ -514,7 +521,7 @@ def get_template_json_data(template_name: str = None, template_path: str or path if template_name and not template_path: template_path = get_registered(template_name=template_name, project_path=project_path) - return get_json_data('template', template_path, validation.valid_o3de_template_json) + return get_json_data('template', template_path, validation.valid_o3de_template_json, template_name) def get_restricted_json_data(restricted_name: str = None, restricted_path: str or pathlib.Path = None, @@ -526,18 +533,18 @@ def get_restricted_json_data(restricted_name: str = None, restricted_path: str o if restricted_name and not restricted_path: restricted_path = get_registered(restricted_name=restricted_name, project_path=project_path) - return get_json_data('restricted', restricted_path, validation.valid_o3de_restricted_json) + return get_json_data('restricted', restricted_path, validation.valid_o3de_restricted_json, restricted_name) -def get_repo_json_data(repo_uri: str = None) -> dict or None: +def get_repo_json_data(repo_uri: str) -> dict or None: if not repo_uri: logger.error('Must specify a Repo Uri.') return None repo_json = get_repo_path(repo_uri=repo_uri) - get_json_data_file("Repo", repo_json, validation.valid_o3de_repo_json) + return get_json_data_file(repo_json, "Repo", validation.valid_o3de_repo_json) -def get_repo_path(repo_uri: str = None, cache_folder: str = None) -> pathlib.Path: +def get_repo_path(repo_uri: str, cache_folder: str = None) -> pathlib.Path: if not cache_folder: cache_folder = get_o3de_cache_folder() From 70053f055a7dbbdd336d7027ca938ba2492f3e4f Mon Sep 17 00:00:00 2001 From: nggieber Date: Fri, 8 Oct 2021 16:52:58 -0700 Subject: [PATCH 4/5] More python PR feedback Signed-off-by: nggieber --- scripts/o3de/o3de/manifest.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index 5d34c4bcf9..c5f6315474 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -423,11 +423,8 @@ def get_templates_for_generic_creation(): # temporary until we have a better wa def get_json_file_path(object_typename: str, object_path: str or pathlib.Path = None) -> pathlib.Path: - if not object_typename: - logger.error('Missing object typename.') - return None - - if not object_path: + if not object_typename or not object_path: + logger.error('Must specify an object typename and object path.') return None object_path = pathlib.Path(object_path).resolve() @@ -439,9 +436,11 @@ def get_json_data_file(object_json: pathlib.Path, object_validator = callable) -> dict or None: if not object_typename: logger.error('Missing object typename.') + return None if not object_json: - logger.error(f'No object_json provided for {object_typename}') + logger.error(f'No object json provided for {object_typename}') + return None if not object_json.is_file(): logger.error(f'{object_typename} json {object_json} is not present.') @@ -449,6 +448,7 @@ def get_json_data_file(object_json: pathlib.Path, if not object_validator: logger.error('Missing object validator.') + return None if not object_validator(object_json): logger.error(f'{object_typename} json {object_json} is not valid.') @@ -472,6 +472,7 @@ def get_json_data(object_typename: str = None, if not object_json and object_name: logger.error(f'{object_name} has not been registered.') + return None return get_json_data_file(object_json, object_typename, object_validator) From 56de6064d2fa7755b9c34335d9a14f1afd934c08 Mon Sep 17 00:00:00 2001 From: nggieber Date: Mon, 11 Oct 2021 12:07:04 -0700 Subject: [PATCH 5/5] Hopefully final pass on PR comments Signed-off-by: nggieber --- scripts/o3de/o3de/manifest.py | 43 ++++++++++++----------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index c5f6315474..b665727a4e 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -422,7 +422,7 @@ def get_templates_for_generic_creation(): # temporary until we have a better wa return list(filter(filter_project_and_gem_templates_out, get_all_templates())) def get_json_file_path(object_typename: str, - object_path: str or pathlib.Path = None) -> pathlib.Path: + object_path: str or pathlib.Path) -> pathlib.Path: if not object_typename or not object_path: logger.error('Must specify an object typename and object path.') return None @@ -432,26 +432,18 @@ def get_json_file_path(object_typename: str, def get_json_data_file(object_json: pathlib.Path, - object_typename: str = None, - object_validator = callable) -> dict or None: + object_typename: str, + object_validator: callable) -> dict or None: if not object_typename: logger.error('Missing object typename.') return None - if not object_json: - logger.error(f'No object json provided for {object_typename}') + if not object_json or not object_json.is_file(): + logger.error(f'Invalid {object_typename} json {object_json} supplied or file missing.') return None - if not object_json.is_file(): - logger.error(f'{object_typename} json {object_json} is not present.') - return None - - if not object_validator: - logger.error('Missing object validator.') - return None - - if not object_validator(object_json): - logger.error(f'{object_typename} json {object_json} is not valid.') + if not object_validator or not object_validator(object_json): + logger.error(f'{object_typename} json {object_json} is not valid or could not be validated.') return None with object_json.open('r') as f: @@ -464,16 +456,11 @@ def get_json_data_file(object_json: pathlib.Path, return None -def get_json_data(object_typename: str = None, - object_path: str or pathlib.Path = None, - object_validator = callable, - object_name: str = None) -> dict or None: +def get_json_data(object_typename: str, + object_path: str or pathlib.Path, + object_validator: callable) -> dict or None: object_json = get_json_file_path(object_typename, object_path) - if not object_json and object_name: - logger.error(f'{object_name} has not been registered.') - return None - return get_json_data_file(object_json, object_typename, object_validator) @@ -486,7 +473,7 @@ def get_engine_json_data(engine_name: str = None, if engine_name and not engine_path: engine_path = get_registered(engine_name=engine_name) - return get_json_data('engine', engine_path, validation.valid_o3de_engine_json, engine_name) + return get_json_data('engine', engine_path, validation.valid_o3de_engine_json) def get_project_json_data(project_name: str = None, @@ -498,7 +485,7 @@ def get_project_json_data(project_name: str = None, if project_name and not project_path: project_path = get_registered(project_name=project_name) - return get_json_data('project', project_path, validation.valid_o3de_project_json, project_name) + return get_json_data('project', project_path, validation.valid_o3de_project_json) def get_gem_json_data(gem_name: str = None, gem_path: str or pathlib.Path = None, @@ -510,7 +497,7 @@ def get_gem_json_data(gem_name: str = None, gem_path: str or pathlib.Path = None if gem_name and not gem_path: gem_path = get_registered(gem_name=gem_name, project_path=project_path) - return get_json_data('gem', gem_path, validation.valid_o3de_gem_json, gem_name) + return get_json_data('gem', gem_path, validation.valid_o3de_gem_json) def get_template_json_data(template_name: str = None, template_path: str or pathlib.Path = None, @@ -522,7 +509,7 @@ def get_template_json_data(template_name: str = None, template_path: str or path if template_name and not template_path: template_path = get_registered(template_name=template_name, project_path=project_path) - return get_json_data('template', template_path, validation.valid_o3de_template_json, template_name) + return get_json_data('template', template_path, validation.valid_o3de_template_json) def get_restricted_json_data(restricted_name: str = None, restricted_path: str or pathlib.Path = None, @@ -534,7 +521,7 @@ def get_restricted_json_data(restricted_name: str = None, restricted_path: str o if restricted_name and not restricted_path: restricted_path = get_registered(restricted_name=restricted_name, project_path=project_path) - return get_json_data('restricted', restricted_path, validation.valid_o3de_restricted_json, restricted_name) + return get_json_data('restricted', restricted_path, validation.valid_o3de_restricted_json) def get_repo_json_data(repo_uri: str) -> dict or None: if not repo_uri: