From 12eef6907e5e8cac22f95d0073081fa84c8c851e Mon Sep 17 00:00:00 2001 From: syntron Date: Wed, 26 Nov 2025 20:21:33 +0100 Subject: [PATCH 1/6] [OMCSession*] define set_timeout() --- OMPython/OMCSession.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index 861f2a3a..46590c06 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -488,8 +488,6 @@ class OMCSessionRunData: cmd_model_executable: Optional[str] = None # additional library search path; this is mainly needed if OMCProcessLocal is run on Windows cmd_library_path: Optional[str] = None - # command timeout - cmd_timeout: Optional[float] = 10.0 # working directory to be used on the *local* system cmd_cwd_local: Optional[str] = None @@ -564,13 +562,12 @@ def omc_run_data_update(self, omc_run_data: OMCSessionRunData) -> OMCSessionRunD """ return self.omc_process.omc_run_data_update(omc_run_data=omc_run_data) - @staticmethod - def run_model_executable(cmd_run_data: OMCSessionRunData) -> int: + def run_model_executable(self, cmd_run_data: OMCSessionRunData) -> int: """ Run the command defined in cmd_run_data. This class is defined as static method such that there is no need to keep instances of over classes around. """ - return OMCSession.run_model_executable(cmd_run_data=cmd_run_data) + return self.omc_process.run_model_executable(cmd_run_data=cmd_run_data) def execute(self, command: str): return self.omc_process.execute(command=command) @@ -727,6 +724,19 @@ def __del__(self): finally: self._omc_process = None + def set_timeout(self, timeout: Optional[float] = None) -> float: + """ + Set the timeout to be used for OMC communication (OMCSession). + + The defined value is set and the current value is returned. If None is provided as argument, nothing is changed. + """ + retval = self._timeout + if timeout is not None: + if timeout <= 0.0: + raise OMCSessionException(f"Invalid timeout value: {timeout}!") + self._timeout = timeout + return retval + @staticmethod def escape_str(value: str) -> str: """ @@ -778,11 +788,9 @@ def omcpath_tempdir(self, tempdir_base: Optional[OMCPath] = None) -> OMCPath: return tempdir - @staticmethod - def run_model_executable(cmd_run_data: OMCSessionRunData) -> int: + def run_model_executable(self, cmd_run_data: OMCSessionRunData) -> int: """ - Run the command defined in cmd_run_data. This class is defined as static method such that there is no need to - keep instances of over classes around. + Run the command defined in cmd_run_data. """ my_env = os.environ.copy() @@ -799,7 +807,7 @@ def run_model_executable(cmd_run_data: OMCSessionRunData) -> int: text=True, env=my_env, cwd=cmd_run_data.cmd_cwd_local, - timeout=cmd_run_data.cmd_timeout, + timeout=self._timeout, check=True, ) stdout = cmdres.stdout.strip() From fb568de409dc295904cadbf316ed782aeb0fae42 Mon Sep 17 00:00:00 2001 From: syntron Date: Tue, 25 Nov 2025 22:26:36 +0100 Subject: [PATCH 2/6] [OMCSession*] align all usages of timeout to the same structure --- OMPython/OMCSession.py | 113 +++++++++++++++++++++-------------------- 1 file changed, 58 insertions(+), 55 deletions(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index 46590c06..32ba5b95 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -839,34 +839,32 @@ def sendExpression(self, command: str, parsed: bool = True) -> Any: Caller should only check for OMCSessionException. """ - # this is needed if the class is not fully initialized or in the process of deletion - if hasattr(self, '_timeout'): - timeout = self._timeout - else: - timeout = 1.0 - if self._omc_zmq is None: raise OMCSessionException("No OMC running. Please create a new instance of OMCSession!") logger.debug("sendExpression(%r, parsed=%r)", command, parsed) + MAX_RETRIES = 50 attempts = 0 - while True: + while attempts < MAX_RETRIES: + attempts += 1 + try: self._omc_zmq.send_string(str(command), flags=zmq.NOBLOCK) break except zmq.error.Again: pass - attempts += 1 - if attempts >= 50: - # in the deletion process, the content is cleared. Thus, any access to a class attribute must be checked - try: - log_content = self.get_log() - except OMCSessionException: - log_content = 'log not available' - raise OMCSessionException(f"No connection with OMC (timeout={timeout}). " - f"Log-file says: \n{log_content}") - time.sleep(timeout / 50.0) + time.sleep(self._timeout / MAX_RETRIES) + else: + # in the deletion process, the content is cleared. Thus, any access to a class attribute must be checked + try: + log_content = self.get_log() + except OMCSessionException: + log_content = 'log not available' + + logger.error(f"Docker did not start. Log-file says:\n{log_content}") + raise OMCSessionException(f"No connection with OMC (timeout={self._timeout}).") + if command == "quit()": self._omc_zmq.close() self._omc_zmq = None @@ -1095,25 +1093,23 @@ def _omc_port_get(self) -> str: port = None # See if the omc server is running + MAX_RETRIES = 80 attempts = 0 - while True: - omc_portfile_path = self._get_portfile_path() + while attempts < MAX_RETRIES: + attempts += 1 + omc_portfile_path = self._get_portfile_path() if omc_portfile_path is not None and omc_portfile_path.is_file(): # Read the port file with open(file=omc_portfile_path, mode='r', encoding="utf-8") as f_p: port = f_p.readline() break - if port is not None: break - - attempts += 1 - if attempts == 80.0: - raise OMCSessionException(f"OMC Server did not start (timeout={self._timeout}). " - f"Could not open file {omc_portfile_path}. " - f"Log-file says:\n{self.get_log()}") - time.sleep(self._timeout / 80.0) + time.sleep(self._timeout / MAX_RETRIES) + else: + logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") + raise OMCSessionException(f"OMC Server did not start (timeout={self._timeout}).") logger.info(f"Local OMC Server is up and running at ZMQ port {port} " f"pid={self._omc_process.pid if isinstance(self._omc_process, subprocess.Popen) else '?'}") @@ -1195,7 +1191,11 @@ def _docker_process_get(self, docker_cid: str) -> Optional[DockerPopen]: raise NotImplementedError("Docker not supported on win32!") docker_process = None - for _ in range(0, 40): + MAX_RETRIES = 40 + attempts = 0 + while attempts < MAX_RETRIES: + attempts += 1 + docker_top = subprocess.check_output(["docker", "top", docker_cid]).decode().strip() docker_process = None for line in docker_top.split("\n"): @@ -1206,10 +1206,12 @@ def _docker_process_get(self, docker_cid: str) -> Optional[DockerPopen]: except psutil.NoSuchProcess as ex: raise OMCSessionException(f"Could not find PID {docker_top} - " "is this a docker instance spawned without --pid=host?") from ex - if docker_process is not None: break - time.sleep(self._timeout / 40.0) + time.sleep(self._timeout / MAX_RETRIES) + else: + logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") + raise OMCSessionException(f"Docker based OMC Server did not start (timeout={self._timeout}).") return docker_process @@ -1231,8 +1233,11 @@ def _omc_port_get(self) -> str: raise OMCSessionException(f"Invalid docker container ID: {self._docker_container_id}") # See if the omc server is running + MAX_RETRIES = 80 attempts = 0 - while True: + while attempts < MAX_RETRIES: + attempts += 1 + omc_portfile_path = self._get_portfile_path() if omc_portfile_path is not None: try: @@ -1243,16 +1248,12 @@ def _omc_port_get(self) -> str: port = output.decode().strip() except subprocess.CalledProcessError: pass - if port is not None: break - - attempts += 1 - if attempts == 80.0: - raise OMCSessionException(f"Docker based OMC Server did not start (timeout={self._timeout}). " - f"Could not open port file {omc_portfile_path}. " - f"Log-file says:\n{self.get_log()}") - time.sleep(self._timeout / 80.0) + time.sleep(self._timeout / MAX_RETRIES) + else: + logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") + raise OMCSessionException(f"Docker based OMC Server did not start (timeout={self._timeout}).") logger.info(f"Docker based OMC Server is up and running at port {port}") @@ -1420,25 +1421,28 @@ def _docker_omc_start(self) -> Tuple[subprocess.Popen, DockerPopen, str]: raise OMCSessionException(f"Invalid content for docker container ID file path: {docker_cid_file}") docker_cid = None - for _ in range(0, 40): + MAX_RETRIES = 40 + attempts = 0 + while attempts < MAX_RETRIES: + attempts += 1 + try: with open(file=docker_cid_file, mode="r", encoding="utf-8") as fh: docker_cid = fh.read().strip() except IOError: pass - if docker_cid: + if docker_cid is not None: break - time.sleep(self._timeout / 40.0) - - if docker_cid is None: + time.sleep(self._timeout / MAX_RETRIES) + else: logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") raise OMCSessionException(f"Docker did not start (timeout={self._timeout} might be too short " "especially if you did not docker pull the image before this command).") docker_process = self._docker_process_get(docker_cid=docker_cid) if docker_process is None: - raise OMCSessionException(f"Docker top did not contain omc process {self._random_string}. " - f"Log-file says:\n{self.get_log()}") + logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") + raise OMCSessionException(f"Docker top did not contain omc process {self._random_string}.") return omc_process, docker_process, docker_cid @@ -1594,8 +1598,11 @@ def _omc_port_get(self) -> str: port = None # See if the omc server is running + MAX_RETRIES = 80 attempts = 0 - while True: + while attempts < MAX_RETRIES: + attempts += 1 + try: omc_portfile_path = self._get_portfile_path() if omc_portfile_path is not None: @@ -1606,16 +1613,12 @@ def _omc_port_get(self) -> str: port = output.decode().strip() except subprocess.CalledProcessError: pass - if port is not None: break - - attempts += 1 - if attempts == 80.0: - raise OMCSessionException(f"WSL based OMC Server did not start (timeout={self._timeout}). " - f"Could not open port file {omc_portfile_path}. " - f"Log-file says:\n{self.get_log()}") - time.sleep(self._timeout / 80.0) + time.sleep(self._timeout / MAX_RETRIES) + else: + logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") + raise OMCSessionException(f"WSL based OMC Server did not start (timeout={self._timeout}).") logger.info(f"WSL based OMC Server is up and running at ZMQ port {port} " f"pid={self._omc_process.pid if isinstance(self._omc_process, subprocess.Popen) else '?'}") From 702208d78d76ac91462459260665a9c01954b990 Mon Sep 17 00:00:00 2001 From: syntron Date: Wed, 26 Nov 2025 19:38:48 +0100 Subject: [PATCH 3/6] [OMCSession*] simplify code for timeout loops --- OMPython/OMCSession.py | 73 +++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index 32ba5b95..b670e928 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -724,6 +724,31 @@ def __del__(self): finally: self._omc_process = None + def _timeout_loop( + self, + timeout: Optional[float] = None, + timestep: float = 0.1, + ): + """ + Helper (using yield) for while loops to check OMC startup / response. The loop is executed as long as True is + returned, i.e. the first False will stop the while loop. + """ + + if timeout is None: + timeout = self._timeout + if timeout <= 0: + raise OMCSessionException(f"Invalid timeout: {timeout}") + + timer = 0.0 + yield True + while True: + timer += timestep + if timer > timeout: + break + time.sleep(timestep) + yield True + yield False + def set_timeout(self, timeout: Optional[float] = None) -> float: """ Set the timeout to be used for OMC communication (OMCSession). @@ -844,17 +869,13 @@ def sendExpression(self, command: str, parsed: bool = True) -> Any: logger.debug("sendExpression(%r, parsed=%r)", command, parsed) - MAX_RETRIES = 50 - attempts = 0 - while attempts < MAX_RETRIES: - attempts += 1 - + loop = self._timeout_loop(timestep=0.05) + while next(loop): try: self._omc_zmq.send_string(str(command), flags=zmq.NOBLOCK) break except zmq.error.Again: pass - time.sleep(self._timeout / MAX_RETRIES) else: # in the deletion process, the content is cleared. Thus, any access to a class attribute must be checked try: @@ -1093,11 +1114,8 @@ def _omc_port_get(self) -> str: port = None # See if the omc server is running - MAX_RETRIES = 80 - attempts = 0 - while attempts < MAX_RETRIES: - attempts += 1 - + loop = self._timeout_loop(timestep=0.1) + while next(loop): omc_portfile_path = self._get_portfile_path() if omc_portfile_path is not None and omc_portfile_path.is_file(): # Read the port file @@ -1106,7 +1124,6 @@ def _omc_port_get(self) -> str: break if port is not None: break - time.sleep(self._timeout / MAX_RETRIES) else: logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") raise OMCSessionException(f"OMC Server did not start (timeout={self._timeout}).") @@ -1191,11 +1208,8 @@ def _docker_process_get(self, docker_cid: str) -> Optional[DockerPopen]: raise NotImplementedError("Docker not supported on win32!") docker_process = None - MAX_RETRIES = 40 - attempts = 0 - while attempts < MAX_RETRIES: - attempts += 1 - + loop = self._timeout_loop(timestep=0.2) + while next(loop): docker_top = subprocess.check_output(["docker", "top", docker_cid]).decode().strip() docker_process = None for line in docker_top.split("\n"): @@ -1208,7 +1222,6 @@ def _docker_process_get(self, docker_cid: str) -> Optional[DockerPopen]: "is this a docker instance spawned without --pid=host?") from ex if docker_process is not None: break - time.sleep(self._timeout / MAX_RETRIES) else: logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") raise OMCSessionException(f"Docker based OMC Server did not start (timeout={self._timeout}).") @@ -1233,11 +1246,8 @@ def _omc_port_get(self) -> str: raise OMCSessionException(f"Invalid docker container ID: {self._docker_container_id}") # See if the omc server is running - MAX_RETRIES = 80 - attempts = 0 - while attempts < MAX_RETRIES: - attempts += 1 - + loop = self._timeout_loop(timestep=0.1) + while next(loop): omc_portfile_path = self._get_portfile_path() if omc_portfile_path is not None: try: @@ -1250,7 +1260,6 @@ def _omc_port_get(self) -> str: pass if port is not None: break - time.sleep(self._timeout / MAX_RETRIES) else: logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") raise OMCSessionException(f"Docker based OMC Server did not start (timeout={self._timeout}).") @@ -1421,11 +1430,8 @@ def _docker_omc_start(self) -> Tuple[subprocess.Popen, DockerPopen, str]: raise OMCSessionException(f"Invalid content for docker container ID file path: {docker_cid_file}") docker_cid = None - MAX_RETRIES = 40 - attempts = 0 - while attempts < MAX_RETRIES: - attempts += 1 - + loop = self._timeout_loop(timestep=0.1) + while next(loop): try: with open(file=docker_cid_file, mode="r", encoding="utf-8") as fh: docker_cid = fh.read().strip() @@ -1433,7 +1439,6 @@ def _docker_omc_start(self) -> Tuple[subprocess.Popen, DockerPopen, str]: pass if docker_cid is not None: break - time.sleep(self._timeout / MAX_RETRIES) else: logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") raise OMCSessionException(f"Docker did not start (timeout={self._timeout} might be too short " @@ -1598,11 +1603,8 @@ def _omc_port_get(self) -> str: port = None # See if the omc server is running - MAX_RETRIES = 80 - attempts = 0 - while attempts < MAX_RETRIES: - attempts += 1 - + loop = self._timeout_loop(timestep=0.1) + while next(loop): try: omc_portfile_path = self._get_portfile_path() if omc_portfile_path is not None: @@ -1615,7 +1617,6 @@ def _omc_port_get(self) -> str: pass if port is not None: break - time.sleep(self._timeout / MAX_RETRIES) else: logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}") raise OMCSessionException(f"WSL based OMC Server did not start (timeout={self._timeout}).") From cce234bd44f34b5964ea9a98ecdb8e0bd93891f1 Mon Sep 17 00:00:00 2001 From: syntron Date: Thu, 27 Nov 2025 10:21:54 +0100 Subject: [PATCH 4/6] [OMCSession] fix definiton of _timeout variable - use set_timeout() checks --- OMPython/OMCSession.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index b670e928..396fb9e4 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -648,7 +648,9 @@ def __init__( """ # store variables - self._timeout = timeout + # set_timeout() is used to define the value of _timeout as it includes additional checks + self._timeout: float + self.set_timeout(timeout=timeout) # generate a random string for this instance of OMC self._random_string = uuid.uuid4().hex # get a temporary directory From ae4711a30a4b15cf65ad2db60ab1aa93d6c37c7b Mon Sep 17 00:00:00 2001 From: syntron Date: Thu, 27 Nov 2025 10:08:25 +0100 Subject: [PATCH 5/6] [OMCSession*] some additional cleanup (mypy / flake8) * remove not needed variable definitions * fix if condition for bool --- OMPython/OMCSession.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index 396fb9e4..c487b758 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -983,7 +983,7 @@ def sendExpression(self, command: str, parsed: bool = True) -> Any: raise OMCSessionException(f"OMC error occurred for 'sendExpression({command}, {parsed}):\n" f"{msg_long_str}") - if parsed is False: + if not parsed: return result try: @@ -1209,7 +1209,6 @@ def _docker_process_get(self, docker_cid: str) -> Optional[DockerPopen]: if sys.platform == 'win32': raise NotImplementedError("Docker not supported on win32!") - docker_process = None loop = self._timeout_loop(timestep=0.2) while next(loop): docker_top = subprocess.check_output(["docker", "top", docker_cid]).decode().strip() @@ -1601,7 +1600,6 @@ def _omc_process_get(self) -> subprocess.Popen: return omc_process def _omc_port_get(self) -> str: - omc_portfile_path: Optional[pathlib.Path] = None port = None # See if the omc server is running From 252af40d3862a9e3a9042407a7731f281356f95c Mon Sep 17 00:00:00 2001 From: syntron Date: Thu, 27 Nov 2025 21:19:13 +0100 Subject: [PATCH 6/6] [OMCSession] move call to set_timeout() to __post_init__ --- OMPython/OMCSession.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/OMPython/OMCSession.py b/OMPython/OMCSession.py index c487b758..e1d1f123 100644 --- a/OMPython/OMCSession.py +++ b/OMPython/OMCSession.py @@ -648,9 +648,7 @@ def __init__( """ # store variables - # set_timeout() is used to define the value of _timeout as it includes additional checks - self._timeout: float - self.set_timeout(timeout=timeout) + self._timeout = timeout # generate a random string for this instance of OMC self._random_string = uuid.uuid4().hex # get a temporary directory @@ -684,6 +682,9 @@ def __post_init__(self) -> None: """ Create the connection to the OMC server using ZeroMQ. """ + # set_timeout() is used to define the value of _timeout as it includes additional checks + self.set_timeout(timeout=self._timeout) + port = self.get_port() if not isinstance(port, str): raise OMCSessionException(f"Invalid content for port: {port}")