Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more autorestart printing #413

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

FreddieAkeroyd
Copy link
Member

Description of work

Additional printing to help trace autorestart test failure

Code Review

  • Is the code of an acceptable quality?
  • Has the author taken into account the multi-threaded nature of the code?
  • Have the changes been recorded appropriately in a PR for release notes?
  • Has the manual system tests spreadsheet been updated?

Functional Tests

  • Do changes function as described? Add comments below that describe the tests performed.

Final steps

  • Reviewer has updated the submodule in the main EPICS repo? See Reviewing work for the subModules of EPICS in the Git workflow page for details.
  • Reviewer has merged the associated PR for the release notes

@@ -172,9 +172,10 @@ def set_autorestart(self, ioc: str, enable: bool):
curr = self._proc.get_autorestart(ioc)
if curr != enable:
# If different to requested then change it
print_and_log(f"Auto-restart for IOC {ioc} is not {enable}")
Copy link
Contributor

@rerpha rerpha Feb 26, 2025

Choose a reason for hiding this comment

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

i think this sounds like an error. is

Suggested change
print_and_log(f"Auto-restart for IOC {ioc} is not {enable}")
print_and_log(f"Auto-restart for IOC {ioc} not set to {enable}, toggling")

better?

Copy link
Member Author

Choose a reason for hiding this comment

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

The function it calls contains print_and_log(f"Toggling auto-restart for IOC {ioc}") so i didn't feel toggling needed mentioning here too. Happy to change is not True to not set to True

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Flash Review
Development

Successfully merging this pull request may close these issues.

2 participants