-
Notifications
You must be signed in to change notification settings - Fork 202
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
Copy build log and artifacts to a permanent location after failures #4601
base: 5.0.x
Are you sure you want to change the base?
Conversation
b306ccd
to
092fcd0
Compare
I am not sure what is the best way to select the build directory so that I can move it to a more permanent location. That is at the moment I am recreating the location of the build path and then copy the directory to the destination path:
|
50d99c3
to
86fe081
Compare
@gkaf89 The For, for example, for So, I think you could create a subdirectory in the permanent storage location that uses the name of the easyconfig file (to keep it simple), and copy the contents of |
Yeah, the thing to copy should be |
1274a2b
to
b1a9da8
Compare
@gkaf89 If you need any help with this, do let us know! |
b1a9da8
to
6bc53e6
Compare
@boegel The commit is ready. I won't have enough time to familiarize myself with the test framework for the The commit can be tested by modifying the configuration options of some easyconfig that uses the system toolchain to cause a failure. For instance I added the option
in |
@gkaf89 There's a problem with the tests, looks like |
The failure is caused because the target location for permanent storage is the same as the source location. The steps I am following to resolve the issue:
|
b34ff11
to
5cfdfd1
Compare
@boegel Some edge cases where uncovered by the tests. The latest commit resolves the issue. I leave it up you if you prefer to move it to version 5. I am not familiar enough with the tests to test the PR extensively. |
@gkaf89 As briefly discussed during the conf call today, it would be good if you could add a test (or enhance an existing one, like Do let us know if you need any help with that! |
e0d3a6e
to
222041d
Compare
222041d
to
c2035a5
Compare
c2035a5
to
bf40ad9
Compare
bf40ad9
to
04f4c50
Compare
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.
Just did a quick scan as I think everything has been discussed already. Just 2 minor improvements
The create_unused_dir function distinguishes between the parent directory and the name of the target subdirectory in 2 separate arguments. This interface requires additional parameter management when combined with newer path management interfaces such as pathlib. - Add a function create_unused_dirs accepting the target paths as a single argument. - Support the simultaneous creation of multiple unused paths with all of them having the same suffix if any of the actually requested paths is unavailable.
Packages are be built in some selected build path (--buildpath), and the logs of successful compilation are then concentrated to some other location for permanent storage (--logfile-format). Logs of failed builds remain in the build path location so that they can be inspected. However, this setup is problematic when building software in HPC jobs. Quite often in HPC systems the build path is set to some fast storage local to the node, like NVME raid mounted on `/tmp` or `/dev/shm` (as suggested in the documentation: https://docs.easybuild.io/configuration/#buildpath). The node storage is often wiped out after the end of a job, so the log files and the artifacts are no longer available after the termination of the job. This commit adds options (--log-error-path and --artifact-error-path) to accumulate error logs and artifacts in some more permanent locations, so that the can be easily inspected after a failed build. By default, neither the logs nor the build artifacts are copied in the permanent location.
Flags for persisting logs and artifacts are now checked during the validation of input options. In case of invalid settings the program terminates early with an error. Integration tests for internal logging functions are deactivated as the logging functions are never reached in normal operation.
- Avoid single character variables names outside list comprehension. - Use a consistent pattern in function calls throughout the pull request. - Use context manager to mock stdout and stderr.
04f4c50
to
b1e815b
Compare
After using the commit for a while, I made a minor interface change. Now the report message mentions the actual path to the artifacts, which speeds up the navigation to the artifact location. |
Please don't force-push in the middle of a review as I can't tell for sure what has changed. Is only the last commit new or did any of the others change as well? |
c9e9963
to
c0cde17
Compare
When copying logs and artifacts from failed builds, mention the full target path instead of its parent. This reduces the number of actions required to access the artifacts (select, paste, and enter).
c0cde17
to
ea723dd
Compare
@Flamefire Sorry, I try to keep the commit history organized. I will try to thing of a way to limit forced pushes next time. All changes start from the |
The files can be build in some selected build path (--buildpath), and the logs of successful compilation are then concentrated to some other location for permanent storage (--logfile-format). Logs of failed builds remain in the build path location so that they can be inspected.
However, this setup is problematic when building software in HPC jobs. Quite often in HPC systems the build path is set to some fast storage local to the node, like NVME raid mounted on
/tmp
or/dev/shm
(as suggested in the documentation: https://docs.easybuild.io/configuration/#buildpath). The node storage is often wiped out after the end of a job, so the log files and the artifacts are no longer available after the termination of the job.This commit adds an option to accumulate errors in some more permanent location, so they can be easily inspected after a failed build.