From 50d1f8c4bec1358ba9d3db02dc50cfff6cc719f0 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Sat, 29 Jan 2022 14:12:22 +0100 Subject: [PATCH 1/5] Optimize validate dir/zip logic - Opt out early if file list differs, no need to check checksums if file list differs - Remove "checksums" from the set instead of a list --- igniter/bootstrap_repos.py | 49 +++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/igniter/bootstrap_repos.py b/igniter/bootstrap_repos.py index 207253cd2d..4303e729c8 100644 --- a/igniter/bootstrap_repos.py +++ b/igniter/bootstrap_repos.py @@ -973,9 +973,19 @@ class BootstrapRepos: for line in checksums_data.split("\n") if line ] + # get list of files in zip minus `checksums` file itself + # and turn in to set to compare against list of files + # from checksum file. If difference exists, something is + # wrong + files_in_zip = set(zip_file.namelist()) + files_in_zip.remove("checksums") + files_in_checksum = {file[1] for file in checksums} + diff = files_in_zip.difference(files_in_checksum) + if diff: + return False, f"Missing files {diff}" + # calculate and compare checksums in the zip file - for file in checksums: - file_name = file[1] + for file_checksum, file_name in checksums: if platform.system().lower() == "windows": file_name = file_name.replace("/", "\\") h = hashlib.sha256() @@ -983,21 +993,9 @@ class BootstrapRepos: h.update(zip_file.read(file_name)) except FileNotFoundError: return False, f"Missing file [ {file_name} ]" - if h.hexdigest() != file[0]: + if h.hexdigest() != file_checksum: return False, f"Invalid checksum on {file_name}" - # get list of files in zip minus `checksums` file itself - # and turn in to set to compare against list of files - # from checksum file. If difference exists, something is - # wrong - files_in_zip = zip_file.namelist() - files_in_zip.remove("checksums") - files_in_zip = set(files_in_zip) - files_in_checksum = {file[1] for file in checksums} - diff = files_in_zip.difference(files_in_checksum) - if diff: - return False, f"Missing files {diff}" - return True, "All ok" @staticmethod @@ -1011,16 +1009,22 @@ class BootstrapRepos: tuple(line.split(":")) for line in checksums_data.split("\n") if line ] - files_in_dir = [ + + # compare file list against list of files from checksum file. + # If difference exists, something is wrong and we invalidate directly + files_in_dir = set( file.relative_to(path).as_posix() for file in path.iterdir() if file.is_file() - ] + ) files_in_dir.remove("checksums") - files_in_dir = set(files_in_dir) files_in_checksum = {file[1] for file in checksums} - for file in checksums: - file_name = file[1] + diff = files_in_dir.difference(files_in_checksum) + if diff: + return False, f"Missing files {diff}" + + # calculate and compare checksums + for file_checksum, file_name in checksums: if platform.system().lower() == "windows": file_name = file_name.replace("/", "\\") try: @@ -1028,11 +1032,8 @@ class BootstrapRepos: except FileNotFoundError: return False, f"Missing file [ {file_name} ]" - if file[0] != current: + if file_checksum != current: return False, f"Invalid checksum on {file_name}" - diff = files_in_dir.difference(files_in_checksum) - if diff: - return False, f"Missing files {diff}" return True, "All ok" From b088e952e61554707412c1086e8a97d32e352878 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Sat, 29 Jan 2022 14:39:00 +0100 Subject: [PATCH 2/5] Only construct info when needed + optimize "silent commands" lookup - Lookup in "silent commands" as `set` - Do "silent commands" lookup only once instead of in the for loop - Move imports closer to where they are needed (e.g. don't import terminal if we don't use it) --- start.py | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/start.py b/start.py index b6c14526f9..228139d4ad 100644 --- a/start.py +++ b/start.py @@ -86,7 +86,7 @@ Todo: Move or remove bootstrapping environments out of the code. Attributes: - silent_commands (list): list of commands for which we won't print OpenPype + silent_commands (set): list of commands for which we won't print OpenPype logo and info header. .. _MongoDB: @@ -202,8 +202,8 @@ from igniter.tools import ( from igniter.bootstrap_repos import OpenPypeVersion # noqa: E402 bootstrap = BootstrapRepos() -silent_commands = ["run", "igniter", "standalonepublisher", - "extractenvironments"] +silent_commands = {"run", "igniter", "standalonepublisher", + "extractenvironments"} def list_versions(openpype_versions: list, local_version=None) -> None: @@ -1057,29 +1057,30 @@ def boot(): _print(" - for modules ...") set_modules_environments() - from openpype import cli - from openpype.lib import terminal as t - from openpype.version import __version__ - assert version_path, "Version path not defined." - info = get_info(use_staging) - info.insert(0, f">>> Using OpenPype from [ {version_path} ]") - t_width = 20 - try: - t_width = os.get_terminal_size().columns - 2 - except (ValueError, OSError): - # running without terminal - pass + # print info when not running scripts defined in 'silent commands' + if all(arg not in silent_commands for arg in sys.argv): + from openpype.lib import terminal as t + from openpype.version import __version__ - _header = f"*** OpenPype [{__version__}] " + info = get_info(use_staging) + info.insert(0, f">>> Using OpenPype from [ {version_path} ]") - info.insert(0, _header + "-" * (t_width - len(_header))) - for i in info: - # don't show for running scripts - if all(item not in sys.argv for item in silent_commands): + t_width = 20 + try: + t_width = os.get_terminal_size().columns - 2 + except (ValueError, OSError): + # running without terminal + pass + + _header = f"*** OpenPype [{__version__}] " + info.insert(0, _header + "-" * (t_width - len(_header))) + + for i in info: t.echo(i) + from openpype import cli try: cli.main(obj={}, prog_name="openpype") except Exception: # noqa From 2373d13348f4d04e2da5063a74e618e696822bd6 Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Sat, 29 Jan 2022 15:13:12 +0100 Subject: [PATCH 3/5] Re-use global settings more since the data is large and query is slow over network (2s+) --- igniter/tools.py | 7 +++---- start.py | 9 +++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/igniter/tools.py b/igniter/tools.py index 735402e9a2..ed33d7c766 100644 --- a/igniter/tools.py +++ b/igniter/tools.py @@ -161,18 +161,17 @@ def get_openpype_global_settings(url: str) -> dict: return global_settings.get("data") or {} -def get_openpype_path_from_db(url: str) -> Union[str, None]: +def get_openpype_path_from_db(settings: dict) -> Union[str, None]: """Get OpenPype path from global settings. Args: - url (str): mongodb url. + settings (dict): mongodb url. Returns: path to OpenPype or None if not found """ - global_settings = get_openpype_global_settings(url) paths = ( - global_settings + settings .get("openpype_path", {}) .get(platform.system().lower()) ) or [] diff --git a/start.py b/start.py index 228139d4ad..3a8ed834eb 100644 --- a/start.py +++ b/start.py @@ -281,12 +281,11 @@ def run(arguments: list, env: dict = None) -> int: return p.returncode -def run_disk_mapping_commands(mongo_url): +def run_disk_mapping_commands(settings): """ Run disk mapping command Used to map shared disk for OP to pull codebase. """ - settings = get_openpype_global_settings(mongo_url) low_platform = platform.system().lower() disk_mapping = settings.get("disk_mapping") @@ -923,12 +922,14 @@ def boot(): os.environ["OPENPYPE_DATABASE_NAME"] = \ os.environ.get("OPENPYPE_DATABASE_NAME") or "openpype" + global_settings = get_openpype_global_settings(openpype_mongo) + _print(">>> run disk mapping command ...") - run_disk_mapping_commands(openpype_mongo) + run_disk_mapping_commands(global_settings) # Get openpype path from database and set it to environment so openpype can # find its versions there and bootstrap them. - openpype_path = get_openpype_path_from_db(openpype_mongo) + openpype_path = get_openpype_path_from_db(global_settings) if getattr(sys, 'frozen', False): local_version = bootstrap.get_version(Path(OPENPYPE_ROOT)) From 04613be3a1f626e7fbbfb2213db1dbbb73b5024e Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Tue, 8 Feb 2022 22:59:44 +0100 Subject: [PATCH 4/5] Remove unused import --- igniter/install_dialog.py | 1 - 1 file changed, 1 deletion(-) diff --git a/igniter/install_dialog.py b/igniter/install_dialog.py index 251adebc9f..b09529f5c5 100644 --- a/igniter/install_dialog.py +++ b/igniter/install_dialog.py @@ -12,7 +12,6 @@ from Qt.QtCore import QTimer # noqa from .install_thread import InstallThread from .tools import ( validate_mongo_connection, - get_openpype_path_from_db, get_openpype_icon_path ) From 86c6e512635fc4a2b927f4dfe83dc22c1de858aa Mon Sep 17 00:00:00 2001 From: Roy Nieterau Date: Tue, 8 Feb 2022 23:05:51 +0100 Subject: [PATCH 5/5] Refactor remaining usage of get_openpype_path_from_db -> get_openpype_path_from_settings --- igniter/bootstrap_repos.py | 6 ++++-- igniter/tools.py | 2 +- start.py | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/igniter/bootstrap_repos.py b/igniter/bootstrap_repos.py index 4303e729c8..ad49f868d5 100644 --- a/igniter/bootstrap_repos.py +++ b/igniter/bootstrap_repos.py @@ -23,7 +23,8 @@ from .user_settings import ( OpenPypeSettingsRegistry ) from .tools import ( - get_openpype_path_from_db, + get_openpype_global_settings, + get_openpype_path_from_settings, get_expected_studio_version_str ) @@ -1263,7 +1264,8 @@ class BootstrapRepos: openpype_path = None # try to get OpenPype path from mongo. if location.startswith("mongodb"): - openpype_path = get_openpype_path_from_db(location) + global_settings = get_openpype_global_settings(location) + openpype_path = get_openpype_path_from_settings(global_settings) if not openpype_path: self._print("cannot find OPENPYPE_PATH in settings.") return None diff --git a/igniter/tools.py b/igniter/tools.py index ed33d7c766..57159b5e52 100644 --- a/igniter/tools.py +++ b/igniter/tools.py @@ -161,7 +161,7 @@ def get_openpype_global_settings(url: str) -> dict: return global_settings.get("data") or {} -def get_openpype_path_from_db(settings: dict) -> Union[str, None]: +def get_openpype_path_from_settings(settings: dict) -> Union[str, None]: """Get OpenPype path from global settings. Args: diff --git a/start.py b/start.py index 3a8ed834eb..f8a01dd9ab 100644 --- a/start.py +++ b/start.py @@ -195,7 +195,7 @@ import igniter # noqa: E402 from igniter import BootstrapRepos # noqa: E402 from igniter.tools import ( get_openpype_global_settings, - get_openpype_path_from_db, + get_openpype_path_from_settings, validate_mongo_connection, OpenPypeVersionNotFound ) # noqa @@ -929,7 +929,7 @@ def boot(): # Get openpype path from database and set it to environment so openpype can # find its versions there and bootstrap them. - openpype_path = get_openpype_path_from_db(global_settings) + openpype_path = get_openpype_path_from_settings(global_settings) if getattr(sys, 'frozen', False): local_version = bootstrap.get_version(Path(OPENPYPE_ROOT))