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
fix(fast xmlupload): make process-files more robust (DEV-2235) #395
Conversation
DEV-2235 DSP-TOOLS: process-files fails with docker.errors.APIError: 502 Server Error (Bad Gateway / Bad response from Docker engine)
On Vijs machine, this happened:
Log fileThe log file's last entry is from 2023-06-01 22:01:55,997. The log file doesn't contain the beginning of the process, because the old entries were already deleted. Disappearing filesThe input folder contains:
The output folder contains:
For some reasons, much more .orig files were written than sidecars and derivates. That's very strange. Also the log files show much more entries for .orig file creation, and very few for derivates and sidecars. Disappearing docker containerUnfortunately, the Docker container cannot be examined any more, because it was stopped. Steps to improve
|
# restart sipi container | ||
_stop_and_remove_sipi_container() | ||
_start_sipi_container_and_mount_volumes(input_dir, output_dir) | ||
global sipi_container | ||
sipi_container = _get_sipi_container() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect candidate to extract into a method restart_sipi_container()
- then you don't even need the comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored these methods, because they were a bit spurious anyway.
orig_filepath_2_uuid += _process_files_in_parallel( | ||
paths=unprocessed_paths, | ||
input_dir=input_dir, | ||
output_dir=output_dir, | ||
nthreads=nthreads | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doing a recursive call like this can potentially be a cause for errors, if the max recursion depth gets reached. In general, it would probably be more robust to move calling this function again one "level out", so it's flat, not recursive.
But, to be honest, I'm not sure how big the danger is of this actually happening
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be safe, I moved calling this function again one level out. Can you check that there's no logical flaw in it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't look thoroughly, but at first glance it seems good
resolves DEV-2235