Skip to content

[ModelicaSystem*] add timeout argument - #382

Merged
adeas31 merged 10 commits into
OpenModelica:masterfrom
syntron:ModelicaSystem_timeout
Jan 29, 2026
Merged

[ModelicaSystem*] add timeout argument#382
adeas31 merged 10 commits into
OpenModelica:masterfrom
syntron:ModelicaSystem_timeout

Conversation

@syntron

Copy link
Copy Markdown
Contributor

Add / fix argument timeout to / of ModelicaSystem and ModelicaSystemDoE

@syntron
syntronforce-pushed the ModelicaSystem_timeout branch from ecd996f to d37ec0bCompareNovember 24, 2025 20:57

@adeas31adeas31 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ModelicaSystemDoE and ModelicaSystem doesn't really need timeout. The timeout is used by OMCSessionZMQ so it is better to expose timeout with API so it can be set to something else other than default 10s.

I also saw that timeout is used very randomly in the code. For starting omc we are using timeout of 2s and then each process class also uses timeout. It is also used for command execution. Perhaps its better to clean up the code and use timeout properly.

@syntron
syntron requested a review from adeas31November 26, 2025 18:40
@syntron

Copy link
Copy Markdown
ContributorAuthor

@adeas31 update the code to:
(1) remove all timeouts from ModelicaSystem* classes
(2) define timeouts in OMCSession*
(3) allow to set timeout value
(4) use one variable for all OMC startup / program execution timeouts
(5) wrap timeout handling in a separate function

@syntron

syntron commented Nov 26, 2025

Copy link
Copy Markdown
ContributorAuthor

move point (1) to PR #383; this PR could be applied now

the remaining changes should be applied after the modifications of OMCSession as it changes the same code and this means only one (final) rebase

@syntron
syntronforce-pushed the ModelicaSystem_timeout branch from f9d995c to cbb6e56CompareNovember 27, 2025 19:45
@syntron

Copy link
Copy Markdown
ContributorAuthor

rebased on top of PR #386

Comment threadOMPython/OMCSession.py Outdated
time.sleep(self._timeout / 80.0)
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}).")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to log the file name?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code is changed such that the name of the log file is not stored anymore - I will change this such that the log file is reported in the exception

Comment threadOMPython/OMCSession.py Outdated
f"Log-file says:\n{self.get_log()}")
time.sleep(self._timeout / 80.0)
else:
logger.error(f"Docker did not start. Log-file says:\n{self.get_log()}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a local server. Not a docker.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace by 'OMC server did not start. Log...'

Comment threadOMPython/OMCSession.py Outdated
time.sleep(self._timeout / 80.0)
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}).")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again the log file name is skipped.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same change as above ...

@github-project-automationgithub-project-automationBot moved this from Backlog to In review in OMPython Release 5.0.0 planJan 27, 2026
@syntron
syntronforce-pushed the ModelicaSystem_timeout branch from 1e9b435 to 96964d7CompareJanuary 27, 2026 19:50
@syntron
syntronforce-pushed the ModelicaSystem_timeout branch from 96964d7 to 981f69aCompareJanuary 27, 2026 20:01
Comment threadOMPython/OMCSession.py Outdated
time.sleep(self._timeout / 80.0)
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}).")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be Windows subsystem instead of docker, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct - fixed

@adeas31
adeas31 enabled auto-merge (squash) January 29, 2026 12:48
@adeas31
adeas31 merged commit d34e223 into OpenModelica:masterJan 29, 2026
14 checks passed
@syntron
syntron deleted the ModelicaSystem_timeout branch January 29, 2026 22:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants

@syntron@adeas31