[BUGFIX] Handle missing log directories more gracefully #1418
Conversation
If an exception is thrown inside of the logging cleanup, this will properly handle it and clean up the job.
There was a problem hiding this comment.
The try here is wrapped with the main ARM ripper code, suggest that if this fails the whole job doesn't need to fail.
Add an additional try/exception before the main, that will just throw an error that the log files are missing and move on
There was a problem hiding this comment.
Hi @microtechno9000. Sorry for the delay on this, but I think it should be updated now. It will now catch/log the error and continue on with the job.
I've also bumped the version.
I also think there may still be some value in expanding the scope of the try: block that calls main(), so that ARM can recover in the case that some earlier part of the job fails in an unexpected way, but I'm not sure where the best place to start it would be...
| for log_dir in logs_folders: | ||
| logging.info(f"Checking path {log_dir} for old log files...") | ||
| # Loop through each file in current folder and remove files older than set in arm.yaml | ||
| if not os.path.isdir(log_dir): |
There was a problem hiding this comment.
no issues with this
|
Thanks for making ARM better, and adding logs to your PR :) The version script isn't working for some reason at the moment, please bump your ARM version manually |
|
VERSION
Outdated
| @@ -1 +1 @@ | |||
| 2.17.1 | |||
| 2.17.2 | |||
There was a problem hiding this comment.
Ok, I merged in the latest from main and updated the version. Let me know if there's anything further I need to do RE the submodule. It's been a while since I used them.
|
Description
I recently set up ARM on a bare metal Ubuntu 25.04 install (I'm hoping to do a writeup soon for that as well). Since there is more manual setup involved, I forgot to create the
logs/progressdirectory, which caused themain.pyscript to crash inside the log cleanup function. When that happened, the exception wasn't logged in any of the logs, as far as I could tell, and the job was left in the database (and couldn't be abandoned since the PID was no longer is running).This makes two small changes to better handle this scenario:
tryblock inmain.py. That way if an exception is thrown during log cleanup, it is caught, logged, and the job is cleaned up properly.This is my first contribution, and I'm happy to adjust the PR if anything isn't following the correct procedures, just let me know!
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
I tested these on bare metal since I don't have docker, but it should be possible to test on docker as well, if you can build an image where you remove the
logs/progressdirectory.Change to log cleanup
logs/progressdirectory does not existChange to top level exception handling
logger.pyto continue when a logging directory is missingChecklist:
Note: there's no functional changes in here, so documentation updates shouldn't be needed.
Changelog:
Include the details of changes made here
logger.pymain.pyso it handles exceptions in log cleanupLogs
Attach logs from successful test runs here
Sample run where the
logs/progressdirectory hasn't been created is below. Before the fix, the job would crash after the "Checking path /home/arm/logs/progress for old log files..." entry.Sample run with the fix in
logger.pycommented out. This would previously crash silently and the job would appear to still be in progress.