From 5e5058c1f28abe595d56378b227ef765f6968234 Mon Sep 17 00:00:00 2001 From: yuanmengqi Date: Tue, 15 Jul 2025 21:38:34 +0000 Subject: [PATCH] fix: standardize provider interface parameters across all implementations - Add screen_size parameter to get_vm_path() for all providers (with default 1920x1080) - Add os_type parameter to start_emulator() for Azure and VirtualBox providers - Add region parameter to stop_emulator() for VMware, Docker, and VirtualBox providers - Use *args, **kwargs for better extensibility and parameter consistency - Add documentation comments explaining ignored parameters for interface consistency - Prevents TypeError exceptions when AWS-specific parameters are passed to other providers This ensures all providers can handle the same parameter sets while maintaining backward compatibility and avoiding interface fragmentation. --- desktop_env/providers/azure/manager.py | 4 +++- desktop_env/providers/azure/provider.py | 4 +++- desktop_env/providers/docker/manager.py | 4 +++- desktop_env/providers/docker/provider.py | 4 +++- desktop_env/providers/virtualbox/manager.py | 4 +++- desktop_env/providers/virtualbox/provider.py | 8 ++++++-- desktop_env/providers/vmware/manager.py | 4 +++- desktop_env/providers/vmware/provider.py | 4 +++- 8 files changed, 27 insertions(+), 9 deletions(-) diff --git a/desktop_env/providers/azure/manager.py b/desktop_env/providers/azure/manager.py index 6076511..6e0bf1d 100644 --- a/desktop_env/providers/azure/manager.py +++ b/desktop_env/providers/azure/manager.py @@ -67,7 +67,9 @@ class AzureVMManager(VMManager): free_vms.append((vm_path, pid_str)) return free_vms - def get_vm_path(self, region): + def get_vm_path(self, region, screen_size=(1920, 1080), **kwargs): + # Note: screen_size parameter is ignored for Azure provider + # but kept for interface consistency with other providers self.check_and_clean() free_vms_paths = self.list_free_vms(region) if len(free_vms_paths) == 0: diff --git a/desktop_env/providers/azure/provider.py b/desktop_env/providers/azure/provider.py index fb43503..b0a489f 100644 --- a/desktop_env/providers/azure/provider.py +++ b/desktop_env/providers/azure/provider.py @@ -32,7 +32,9 @@ class AzureProvider(Provider): self.compute_client = ComputeManagementClient(credential, self.subscription_id) self.network_client = NetworkManagementClient(credential, self.subscription_id) - def start_emulator(self, path_to_vm: str, headless: bool): + def start_emulator(self, path_to_vm: str, headless: bool, os_type: str = None, *args, **kwargs): + # Note: os_type parameter is ignored for Azure provider + # but kept for interface consistency with other providers logger.info("Starting Azure VM...") resource_group_name, vm_name = path_to_vm.split('/') diff --git a/desktop_env/providers/docker/manager.py b/desktop_env/providers/docker/manager.py index 2461a9a..fcc6a68 100644 --- a/desktop_env/providers/docker/manager.py +++ b/desktop_env/providers/docker/manager.py @@ -105,7 +105,9 @@ class DockerVMManager(VMManager): def occupy_vm(self, vm_path): pass - def get_vm_path(self, os_type, region): + def get_vm_path(self, os_type, region, screen_size=(1920, 1080), **kwargs): + # Note: screen_size parameter is ignored for Docker provider + # but kept for interface consistency with other providers global URL, DOWNLOADED_FILE_NAME if os_type == "Ubuntu": URL = UBUNTU_X86_URL diff --git a/desktop_env/providers/docker/provider.py b/desktop_env/providers/docker/provider.py index 5c58b27..6fb1ee6 100644 --- a/desktop_env/providers/docker/provider.py +++ b/desktop_env/providers/docker/provider.py @@ -144,7 +144,9 @@ class DockerProvider(Provider): def revert_to_snapshot(self, path_to_vm: str, snapshot_name: str): self.stop_emulator(path_to_vm) - def stop_emulator(self, path_to_vm: str): + def stop_emulator(self, path_to_vm: str, region=None, *args, **kwargs): + # Note: region parameter is ignored for Docker provider + # but kept for interface consistency with other providers if self.container: logger.info("Stopping VM...") try: diff --git a/desktop_env/providers/virtualbox/manager.py b/desktop_env/providers/virtualbox/manager.py index b407ac8..3fa053e 100644 --- a/desktop_env/providers/virtualbox/manager.py +++ b/desktop_env/providers/virtualbox/manager.py @@ -428,7 +428,9 @@ class VirtualBoxVMManager(VMManager): free_vms.append((vm_path, pid_str)) return free_vms - def get_vm_path(self, os_type, region=None): + def get_vm_path(self, os_type, region=None, screen_size=(1920, 1080), **kwargs): + # Note: screen_size parameter is ignored for VirtualBox provider + # but kept for interface consistency with other providers if os_type != "Ubuntu": raise ValueError("Only support Ubuntu for now.") diff --git a/desktop_env/providers/virtualbox/provider.py b/desktop_env/providers/virtualbox/provider.py index 81cda08..ad64703 100644 --- a/desktop_env/providers/virtualbox/provider.py +++ b/desktop_env/providers/virtualbox/provider.py @@ -55,7 +55,9 @@ class VirtualBoxProvider(Provider): logger.error(f"Error executing command: {e.output.decode().strip()}") - def start_emulator(self, path_to_vm: str, headless: bool): + def start_emulator(self, path_to_vm: str, headless: bool, os_type: str = None, *args, **kwargs): + # Note: os_type parameter is ignored for VirtualBox provider + # but kept for interface consistency with other providers logger.info("Starting VirtualBox VM...") while True: @@ -113,7 +115,9 @@ class VirtualBoxProvider(Provider): time.sleep(WAIT_TIME) # Wait for the VM to revert return path_to_vm - def stop_emulator(self, path_to_vm: str): + def stop_emulator(self, path_to_vm: str, region=None, *args, **kwargs): + # Note: region parameter is ignored for VirtualBox provider + # but kept for interface consistency with other providers logger.info("Stopping VirtualBox VM...") uuid = VirtualBoxProvider._get_vm_uuid(path_to_vm) VirtualBoxProvider._execute_command(["VBoxManage", "controlvm", uuid, "savestate"]) diff --git a/desktop_env/providers/vmware/manager.py b/desktop_env/providers/vmware/manager.py index f3384fd..806232e 100644 --- a/desktop_env/providers/vmware/manager.py +++ b/desktop_env/providers/vmware/manager.py @@ -417,7 +417,9 @@ class VMwareVMManager(VMManager): free_vms.append((vm_path, pid_str)) return free_vms - def get_vm_path(self, os_type, region=None): + def get_vm_path(self, os_type, region=None, screen_size=(1920, 1080), **kwargs): + # Note: screen_size parameter is ignored for VMware provider + # but kept for interface consistency with other providers with self.lock: if not VMwareVMManager.checked_and_cleaned: VMwareVMManager.checked_and_cleaned = True diff --git a/desktop_env/providers/vmware/provider.py b/desktop_env/providers/vmware/provider.py index 44f8fc3..0902abe 100644 --- a/desktop_env/providers/vmware/provider.py +++ b/desktop_env/providers/vmware/provider.py @@ -97,7 +97,9 @@ class VMwareProvider(Provider): time.sleep(WAIT_TIME) # Wait for the VM to revert return path_to_vm - def stop_emulator(self, path_to_vm: str): + def stop_emulator(self, path_to_vm: str, region=None, *args, **kwargs): + # Note: region parameter is ignored for VMware provider + # but kept for interface consistency with other providers logger.info("Stopping VMware VM...") VMwareProvider._execute_command(["vmrun"] + get_vmrun_type(return_list=True) + ["stop", path_to_vm]) time.sleep(WAIT_TIME) # Wait for the VM to stop