From 9757ba9276adea4c17e561f034cca3c51ac8b168 Mon Sep 17 00:00:00 2001 From: Brett Date: Sun, 28 Jan 2024 10:30:57 -0600 Subject: [PATCH] Pylint cleanup (#74) * Misc fixes * Misc cleanup * Add all_versions to blender_downloader.py * More cleanup * Fix issue with status not reporting engine info * Misc fixes * Misc cleanup * Add all_versions to blender_downloader.py * More cleanup * Fix issue with status not reporting engine info --- .pylintrc | 2 +- setup.py | 1 + src/api/api_server.py | 15 ++++++------ src/api/serverproxy_manager.py | 2 +- src/engines/blender/blender_downloader.py | 29 +++++++++++++++++++++-- src/engines/blender/blender_worker.py | 3 +-- src/engines/core/base_worker.py | 4 ++-- src/engines/engine_manager.py | 2 +- src/engines/ffmpeg/ffmpeg_downloader.py | 8 +++---- src/engines/ffmpeg/ffmpeg_engine.py | 13 +++++----- src/ui/engine_browser.py | 14 +++-------- src/ui/main_window.py | 15 ++++-------- src/ui/widgets/dialog.py | 1 - src/utilities/misc_helper.py | 27 +++++++++++++++------ 14 files changed, 79 insertions(+), 57 deletions(-) delete mode 100644 src/ui/widgets/dialog.py diff --git a/.pylintrc b/.pylintrc index bdc359d..f097d1e 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,4 +1,4 @@ [MASTER] max-line-length = 120 [MESSAGES CONTROL] -disable = missing-docstring, invalid-name, import-error \ No newline at end of file +disable = missing-docstring, invalid-name, import-error, logging-fstring-interpolation \ No newline at end of file diff --git a/setup.py b/setup.py index b9fc1f1..0e6007c 100644 --- a/setup.py +++ b/setup.py @@ -18,4 +18,5 @@ setup( data_files=DATA_FILES, options={'py2app': OPTIONS}, setup_requires=['py2app'], + name='Zordon' ) diff --git a/src/api/api_server.py b/src/api/api_server.py index e855043..55b6ec4 100755 --- a/src/api/api_server.py +++ b/src/api/api_server.py @@ -60,8 +60,7 @@ def jobs_json(): job_cache_token = num_to_alphanumeric(job_cache_int) if hash_token and hash_token == job_cache_token: return [], 204 # no need to update - else: - return {'jobs': all_jobs, 'token': job_cache_token} + return {'jobs': all_jobs, 'token': job_cache_token} except Exception as e: logger.exception(f"Exception fetching all_jobs_cached: {e}") return [], 500 @@ -343,10 +342,10 @@ def clear_history(): def status(): renderer_data = {} for render_class in EngineManager.supported_engines(): - if EngineManager.all_versions_for_engine(render_class.name): # only return renderers installed on host - renderer_data[render_class.engine.name()] = \ - {'versions': EngineManager.all_versions_for_engine(render_class.engine.name()), - 'is_available': RenderQueue.is_available_for_job(render_class.engine.name()) + if EngineManager.all_versions_for_engine(render_class.name()): # only return renderers installed on host + renderer_data[render_class.name()] = \ + {'versions': EngineManager.all_versions_for_engine(render_class.name()), + 'is_available': RenderQueue.is_available_for_job(render_class.name()) } # Get system info @@ -376,8 +375,8 @@ def renderer_info(): if installed_versions: # fixme: using system versions only because downloaded versions may have permissions issues system_installed_versions = [x for x in installed_versions if x['type'] == 'system'] - install_path = system_installed_versions[0]['path'] if system_installed_versions else ( - installed_versions)[0]['path'] + install_path = system_installed_versions[0]['path'] if system_installed_versions \ + else (installed_versions)[0]['path'] renderer_data[engine.name()] = {'is_available': RenderQueue.is_available_for_job(engine.name()), 'versions': installed_versions, 'supported_extensions': engine.supported_extensions(), diff --git a/src/api/serverproxy_manager.py b/src/api/serverproxy_manager.py index dcd4d7b..7e94a8c 100644 --- a/src/api/serverproxy_manager.py +++ b/src/api/serverproxy_manager.py @@ -17,7 +17,7 @@ class ServerProxyManager: pub.subscribe(cls.__zeroconf_state_change, 'zeroconf_state_change') @classmethod - def __zeroconf_state_change(cls, hostname, state_change, info): + def __zeroconf_state_change(cls, hostname, state_change): if state_change == ServiceStateChange.Added or state_change == ServiceStateChange.Updated: cls.get_proxy_for_hostname(hostname) else: diff --git a/src/engines/blender/blender_downloader.py b/src/engines/blender/blender_downloader.py index 86c5231..7f2b391 100644 --- a/src/engines/blender/blender_downloader.py +++ b/src/engines/blender/blender_downloader.py @@ -1,5 +1,6 @@ import logging import re +import threading import requests @@ -15,6 +16,7 @@ supported_formats = ['.zip', '.tar.xz', '.dmg'] class BlenderDownloader(EngineDownloader): + engine = Blender @staticmethod @@ -79,6 +81,31 @@ class BlenderDownloader(EngineDownloader): return lts_versions + @classmethod + def all_versions(cls, system_os=None, cpu=None): + majors = cls.__get_major_versions() + all_versions = [] + threads = [] + results = [[] for _ in majors] + + def thread_function(major_version, index, system_os, cpu): + results[index] = cls.__get_minor_versions(major_version, system_os, cpu) + + for i, m in enumerate(majors): + thread = threading.Thread(target=thread_function, args=(m, i, system_os, cpu)) + threads.append(thread) + thread.start() + + # Wait for all threads to complete + for thread in threads: + thread.join() + + # Extend all_versions with the results from each thread + for result in results: + all_versions.extend(result) + + return all_versions + @classmethod def find_most_recent_version(cls, system_os=None, cpu=None, lts_only=False): try: @@ -108,8 +135,6 @@ class BlenderDownloader(EngineDownloader): major_version = '.'.join(version.split('.')[:2]) minor_versions = [x for x in cls.__get_minor_versions(major_version, system_os, cpu) if x['version'] == version] - # we get the URL instead of calculating it ourselves. May change this - cls.download_and_extract_app(remote_url=minor_versions[0]['url'], download_location=download_location, timeout=timeout) except IndexError: diff --git a/src/engines/blender/blender_worker.py b/src/engines/blender/blender_worker.py index 2e53a10..dc15639 100644 --- a/src/engines/blender/blender_worker.py +++ b/src/engines/blender/blender_worker.py @@ -12,8 +12,7 @@ class BlenderRenderWorker(BaseRenderWorker): engine = Blender def __init__(self, input_path, output_path, engine_path, args=None, parent=None, name=None): - super(BlenderRenderWorker, self).__init__(input_path=input_path, output_path=output_path, - engine_path=engine_path, args=args, parent=parent, name=name) + super(BlenderRenderWorker, self).__init__(input_path=input_path, output_path=output_path, engine_path=engine_path, args=args, parent=parent, name=name) # Args self.blender_engine = self.args.get('engine', 'BLENDER_EEVEE').upper() diff --git a/src/engines/core/base_worker.py b/src/engines/core/base_worker.py index 7ef1ba8..f5132b4 100644 --- a/src/engines/core/base_worker.py +++ b/src/engines/core/base_worker.py @@ -157,14 +157,14 @@ class BaseRenderWorker(Base): if not os.path.exists(self.input_path): self.status = RenderStatus.ERROR - msg = 'Cannot find input path: {}'.format(self.input_path) + msg = f'Cannot find input path: {self.input_path}' logger.error(msg) self.errors.append(msg) return if not os.path.exists(self.renderer_path): self.status = RenderStatus.ERROR - msg = 'Cannot find render engine path for {}'.format(self.engine.name()) + msg = f'Cannot find render engine path for {self.engine.name()}' logger.error(msg) self.errors.append(msg) return diff --git a/src/engines/engine_manager.py b/src/engines/engine_manager.py index c812b4a..2274557 100644 --- a/src/engines/engine_manager.py +++ b/src/engines/engine_manager.py @@ -239,7 +239,7 @@ class EngineManager: @classmethod def engine_for_project_path(cls, path): - name, extension = os.path.splitext(path) + _, extension = os.path.splitext(path) extension = extension.lower().strip('.') for engine in cls.supported_engines(): if extension in engine.supported_extensions(): diff --git a/src/engines/ffmpeg/ffmpeg_downloader.py b/src/engines/ffmpeg/ffmpeg_downloader.py index 8694ad5..72e75cc 100644 --- a/src/engines/ffmpeg/ffmpeg_downloader.py +++ b/src/engines/ffmpeg/ffmpeg_downloader.py @@ -90,7 +90,7 @@ class FFMPEGDownloader(EngineDownloader): return releases @classmethod - def __all_versions(cls, system_os=None, cpu=None): + def all_versions(cls, system_os=None, cpu=None): system_os = system_os or current_system_os() cpu = cpu or current_system_cpu() versions_per_os = {'linux': cls.__get_linux_versions, 'macos': cls.__get_macos_versions, @@ -131,14 +131,14 @@ class FFMPEGDownloader(EngineDownloader): try: system_os = system_os or current_system_os() cpu = cpu or current_system_cpu() - return cls.__all_versions(system_os, cpu)[0] + return cls.all_versions(system_os, cpu)[0] except (IndexError, requests.exceptions.RequestException): logger.error(f"Cannot get most recent version of ffmpeg") return {} @classmethod def version_is_available_to_download(cls, version, system_os=None, cpu=None): - for ver in cls.__all_versions(system_os, cpu): + for ver in cls.all_versions(system_os, cpu): if ver['version'] == version: return ver return None @@ -149,7 +149,7 @@ class FFMPEGDownloader(EngineDownloader): cpu = cpu or current_system_cpu() # Verify requested version is available - found_version = [item for item in cls.__all_versions(system_os, cpu) if item['version'] == version] + found_version = [item for item in cls.all_versions(system_os, cpu) if item['version'] == version] if not found_version: logger.error(f"Cannot find FFMPEG version {version} for {system_os} and {cpu}") return diff --git a/src/engines/ffmpeg/ffmpeg_engine.py b/src/engines/ffmpeg/ffmpeg_engine.py index 6cf4622..5e780a6 100644 --- a/src/engines/ffmpeg/ffmpeg_engine.py +++ b/src/engines/ffmpeg/ffmpeg_engine.py @@ -25,7 +25,7 @@ class FFMPEG(BaseRenderEngine): def supported_extensions(cls): help_text = (subprocess.check_output([cls().renderer_path(), '-h', 'full'], stderr=subprocess.STDOUT) .decode('utf-8')) - found = re.findall('extensions that .* is allowed to access \(default "(.*)"', help_text) + found = re.findall(r'extensions that .* is allowed to access \(default "(.*)"', help_text) found_extensions = set() for match in found: found_extensions.update(match.split(',')) @@ -36,7 +36,7 @@ class FFMPEG(BaseRenderEngine): try: ver_out = subprocess.check_output([self.renderer_path(), '-version'], timeout=SUBPROCESS_TIMEOUT).decode('utf-8') - match = re.match(".*version\s*([\w.*]+)\W*", ver_out) + match = re.match(r".*version\s*([\w.*]+)\W*", ver_out) if match: version = match.groups()[0] except Exception as e: @@ -50,8 +50,8 @@ class FFMPEG(BaseRenderEngine): 'ffprobe', '-v', 'quiet', '-print_format', 'json', '-show_streams', '-select_streams', 'v', project_path ] - result = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - video_info = json.loads(result.stdout) + output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, text=True) + video_info = json.loads(output) # Extract the necessary information video_stream = video_info['streams'][0] @@ -94,7 +94,7 @@ class FFMPEG(BaseRenderEngine): try: formats_raw = subprocess.check_output([self.renderer_path(), '-formats'], stderr=subprocess.DEVNULL, timeout=SUBPROCESS_TIMEOUT).decode('utf-8') - pattern = '(?P[DE]{1,2})\s+(?P\S{2,})\s+(?P.*)' + pattern = r'(?P[DE]{1,2})\s+(?P\S{2,})\s+(?P.*)' all_formats = [m.groupdict() for m in re.finditer(pattern, formats_raw)] return all_formats except Exception as e: @@ -124,6 +124,7 @@ class FFMPEG(BaseRenderEngine): if match: frame_number = int(match[-1]) return frame_number + return -1 def get_arguments(self): help_text = (subprocess.check_output([self.renderer_path(), '-h', 'long'], stderr=subprocess.STDOUT) @@ -154,4 +155,4 @@ class FFMPEG(BaseRenderEngine): if __name__ == "__main__": - print(FFMPEG().supported_extensions()) + print(FFMPEG().get_all_formats()) diff --git a/src/ui/engine_browser.py b/src/ui/engine_browser.py index 85c5df9..7e23a29 100644 --- a/src/ui/engine_browser.py +++ b/src/ui/engine_browser.py @@ -12,7 +12,7 @@ from PyQt6.QtWidgets import ( from src.api.server_proxy import RenderServerProxy from src.engines.engine_manager import EngineManager -from src.utilities.misc_helper import is_localhost +from src.utilities.misc_helper import is_localhost, launch_url class EngineBrowserWindow(QMainWindow): @@ -98,7 +98,7 @@ class EngineBrowserWindow(QMainWindow): return table_data = [] # convert the data into a flat list - for engine_name, engine_data in raw_server_data.items(): + for _, engine_data in raw_server_data.items(): table_data.extend(engine_data['versions']) self.engine_data = table_data @@ -144,15 +144,7 @@ class EngineBrowserWindow(QMainWindow): def launch_button_click(self): engine_info = self.engine_data[self.table_widget.currentRow()] - path = engine_info['path'] - if sys.platform.startswith('darwin'): - subprocess.run(['open', path]) - elif sys.platform.startswith('win32'): - os.startfile(path) - elif sys.platform.startswith('linux'): - subprocess.run(['xdg-open', path]) - else: - raise OSError("Unsupported operating system") + launch_url(engine_info['path']) def install_button_click(self): self.update_download_status() diff --git a/src/ui/main_window.py b/src/ui/main_window.py index db3c510..36bb6f7 100644 --- a/src/ui/main_window.py +++ b/src/ui/main_window.py @@ -29,6 +29,7 @@ from .widgets.proportional_image_label import ProportionalImageLabel from .widgets.statusbar import StatusBar from .widgets.toolbar import ToolBar from src.api.serverproxy_manager import ServerProxyManager +from src.utilities.misc_helper import launch_url logger = logging.getLogger() @@ -72,7 +73,7 @@ class MainWindow(QMainWindow): # Create a QLabel widget to display the image self.image_label = ProportionalImageLabel() self.image_label.setMaximumSize(700, 500) - self.image_label.setFixedHeight(500) + self.image_label.setFixedHeight(300) self.image_label.setAlignment(Qt.AlignmentFlag.AlignTop | Qt.AlignmentFlag.AlignHCenter) self.load_image_path(os.path.join(resources_dir(), 'Rectangle.png')) @@ -305,7 +306,7 @@ class MainWindow(QMainWindow): except ConnectionError as e: logger.error(f"Connection error fetching image: {e}") except Exception as e: - logger.exception(f"Error fetching image: {e}") + logger.error(f"Error fetching image: {e}") job_id = self.selected_job_ids()[0] if self.selected_job_ids() else None local_server = is_localhost(self.current_hostname) @@ -563,15 +564,7 @@ class MainWindow(QMainWindow): for job_id in job_ids: job_info = self.current_server_proxy.get_job_info(job_id) path = os.path.dirname(job_info['output_path']) - - if sys.platform.startswith('darwin'): - subprocess.run(['open', path]) - elif sys.platform.startswith('win32'): - os.startfile(path) - elif sys.platform.startswith('linux'): - subprocess.run(['xdg-open', path]) - else: - raise OSError("Unsupported operating system") + launch_url(path) def new_job(self) -> None: diff --git a/src/ui/widgets/dialog.py b/src/ui/widgets/dialog.py deleted file mode 100644 index ad23363..0000000 --- a/src/ui/widgets/dialog.py +++ /dev/null @@ -1 +0,0 @@ -''' app/ui/widgets/dialog.py ''' diff --git a/src/utilities/misc_helper.py b/src/utilities/misc_helper.py index 777c261..ae17899 100644 --- a/src/utilities/misc_helper.py +++ b/src/utilities/misc_helper.py @@ -11,14 +11,27 @@ logger = logging.getLogger() def launch_url(url): - if subprocess.run(['which', 'xdg-open'], capture_output=True).returncode == 0: - subprocess.run(['xdg-open', url]) # linux - elif subprocess.run(['which', 'open'], capture_output=True).returncode == 0: - subprocess.run(['open', url]) # macos - elif subprocess.run(['which', 'start'], capture_output=True).returncode == 0: - subprocess.run(['start', url]) # windows - need to validate this works + logger = logging.getLogger(__name__) + + if shutil.which('xdg-open'): + opener = 'xdg-open' + elif shutil.which('open'): + opener = 'open' + elif shutil.which('cmd'): + opener = 'start' else: - logger.error(f"No valid launchers found to launch url: {url}") + error_message = f"No valid launchers found to launch URL: {url}" + logger.error(error_message) + raise OSError(error_message) + + try: + if opener == 'start': + # For Windows, use 'cmd /c start' + subprocess.run(['cmd', '/c', 'start', url], shell=False) + else: + subprocess.run([opener, url]) + except Exception as e: + logger.error(f"Failed to launch URL: {url}. Error: {e}") def file_exists_in_mounts(filepath):