Skip to content

Fix to logFinalizer: adding wnVO#250

Merged
fstagni merged 1 commit intoDIRACGrid:develfrom
Robin-Van-de-Merghel:robin-send-message-fix
Mar 21, 2025
Merged

Fix to logFinalizer: adding wnVO#250
fstagni merged 1 commit intoDIRACGrid:develfrom
Robin-Van-de-Merghel:robin-send-message-fix

Conversation

@Robin-Van-de-Merghel
Copy link
Copy Markdown
Contributor

Related to #249 , added wnVO in sendMessage by adding to the remote logger field.

@Robin-Van-de-Merghel Robin-Van-de-Merghel changed the base branch from master to devel March 20, 2025 13:28
@fstagni
Copy link
Copy Markdown
Contributor

fstagni commented Mar 20, 2025

@martynia can you confirm this?

@martynia
Copy link
Copy Markdown
Contributor

martynia commented Mar 21, 2025

Yes, it is clearly a bug. Thank you for pointing it out. We introduced it with #230. It affects the finaliser as a result of which log files are not renamed (.log added), but still accessible by the client, although most likely not in their complete form.

The pilots are not affected by the bug. If the sys.exit() code is not zero, a pilot has crashed already ;-)
If the code is 0

sys.exit(0)

then the pilot finishes fine (as shown in #249) but the file is not "finalised".
Note: The finaliser relies on sys.exit(0) statement in the last command in the list of commands, otherwise the log will not be finalised.

@fstagni fstagni merged commit d680770 into DIRACGrid:devel Mar 21, 2025
11 checks passed
@Robin-Van-de-Merghel Robin-Van-de-Merghel deleted the robin-send-message-fix branch March 21, 2025 14:29
@Robin-Van-de-Merghel
Copy link
Copy Markdown
Contributor Author

@fstagni I did not know if you wanted to merge into master or into devel to fix it (while there is only one change in the devel branch)

@fstagni
Copy link
Copy Markdown
Contributor

fstagni commented Mar 21, 2025

always merging to devel first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants