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

use more granular exit codes when EasyBuildError is raised #4534

Merged
merged 38 commits into from
Sep 18, 2024

Conversation

dagonzalezfo
Copy link

#4426
This PR will provide a basic set of error codes for EB. The included ones can be classified in 2 groups

  • Eb proper codes:
    • 3: Missing easyconfig
    • 4: No specific easyblock
    • 5: Failed to import easyblock
    • 6: Failed to import class from easyblock
    • 7: No recipe for dependency (robot)
    • 8: Os dependency missing
    • 9: Unavailable sources
    • 10: Sanity check folder/file
    • 11: Can not write module at modules path
    • 12: Validate errors (this is more focus on syntax)
  • Other error codes:
    • HTTP: Errors for fetching sources
    • system error codes for system calls
    • LMOD: Errors for loading module

@smoors smoors added the EasyBuild-5.0 EasyBuild 5.0 label May 21, 2024
@boegel boegel added the change label May 21, 2024
@boegel boegel added this to the 5.0 milestone May 21, 2024
@lexming lexming self-assigned this May 29, 2024
@boegel boegel added the EasyBuild-5.0-blocker Blocker for EasyBuild 5.0 label Jun 5, 2024
Copy link
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

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

I tested this PR and went ahead expanding it to cover more errors. I also created a new Enum class to gather the table of exit codes.

Please check PR dagonzalezfo#1 and specifically commit dagonzalezfo@0d31d34 with my proposed changes.

One needed change is that EasyBuild cannot forward any exit codes from HTTP requests, or from external processes, as those can be out of range (3, 128) or have the same value as the custom exit codes of EB, which would be confusing. EasyBuild should always return its own exit codes and HTTP responses or exit codes from external processes should be reported in the logs.

Example behaviour with my PR:

$ eb zlib-1.2.13-GCCcore-12.3.0.eb
[...]
== configuring...
  >> running shell command:
        potato --prefix=/home/lexming/easybuild/install/software/zlib/1.2.13-GCCcore-12.3.0
        [started at: 2024-06-11 01:51:20]
        [working dir: /home/lexming/.local/easybuild/build/zlib/1.2.13/GCCcore-12.3.0/zlib-1.2.13]
        [output and state saved to /tmp/eb-8b28i2tz/run-shell-cmd-output/potato-coba1es3]

ERROR: Shell command failed!
    full command              ->  potato --prefix=/home/lexming/easybuild/install/software/zlib/1.2.13-GCCcore-12.3.0
    exit code                 ->  127
    working directory         ->  /home/lexming/.local/easybuild/build/zlib/1.2.13/GCCcore-12.3.0/zlib-1.2.13
    output (stdout + stderr)  ->  /tmp/eb-8b28i2tz/run-shell-cmd-output/potato-coba1es3/out.txt
    called from               ->  'configure_step' function in /home/lexming/.local/venv/eb/lib64/python3.10/site-packages/easybuild/easyblocks/generic/configuremake.py (line 326)

== ... (took < 1 sec)
== FAILED: Installation ended unsuccessfully: shell command 'potato ...' failed with exit code 127 in configure step for zlib-1.2.13-GCCcore-12.3.0.eb (took 0 secs)
== Results of the build can be found in the log file(s) /tmp/eb-8b28i2tz/easybuild-zlib-1.2.13-20240611.015120.QzdZC.log
ERROR: Installation of zlib-1.2.13-GCCcore-12.3.0.eb failed: "shell command 'potato ...' failed with exit code 127 in configure step for zlib-1.2.13-GCCcore-12.3.0.eb"
$ echo $?
15

@hvelab
Copy link

hvelab commented Jul 31, 2024

Hi @lexming ,

As Danilo will be out for still some weeks more I will take care of this. Your enhancement looks impressive! About the forwarding of HTTP error codes, those are already catched in these lines as Danilo already thought of this:

https://github.com/dagonzalezfo/easybuild-framework/blob/0d31d34cb5807dd485daf349c847b41f59ce2052/easybuild/tools/filetools.py#L828

If I misunderstood you or you would like these errors to be more granular I can take a look.

@lexming
Copy link
Contributor

lexming commented Aug 2, 2024

@hvelab printing HTTP error codes in warning messages or the logs (which is what that code snippet does) is fine. What is wrong is to use those as exit code for EB as those can easily be out of range (3, 128). This is why I changed the exit code for EB in that case to well-defined EasyBuildExit.FAIL_DOWNLOAD. See a few lines further:

https://github.com/dagonzalezfo/easybuild-framework/blob/0d31d34cb5807dd485daf349c847b41f59ce2052/easybuild/tools/filetools.py#L847

@hvelab
Copy link

hvelab commented Aug 5, 2024

@lexming Great! Then, is there something left to do with this PR from Danilo's part? Please let us know if we can help in any way with your enhancement

@boegel
Copy link
Member

boegel commented Aug 19, 2024

@hvelab We need to get dagonzalezfo#1 merged, which will update this PR.

@lexming I think we can do this without having @dagonzalezfo around though?

@dagonzalezfo
Copy link
Author

Hi, I will take care of it along this week :)

@dagonzalezfo dagonzalezfo requested a review from lexming August 20, 2024 10:49
@lexming lexming changed the title WIP:More granular exit codes More granular exit codes Sep 2, 2024
lexming
lexming previously approved these changes Sep 2, 2024
Copy link
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

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

LGTM

@dagonzalezfo
Copy link
Author

dagonzalezfo commented Sep 2, 2024

Dear @lexming, thanks for your help and improvement on this

easybuild/tools/build_log.py Outdated Show resolved Hide resolved
easybuild/tools/build_log.py Outdated Show resolved Hide resolved
easybuild/tools/build_log.py Outdated Show resolved Hide resolved
easybuild/tools/build_log.py Outdated Show resolved Hide resolved
easybuild/tools/build_log.py Outdated Show resolved Hide resolved
easybuild/tools/github.py Outdated Show resolved Hide resolved
easybuild/tools/options.py Outdated Show resolved Hide resolved
easybuild/tools/options.py Outdated Show resolved Hide resolved
easybuild/tools/options.py Outdated Show resolved Hide resolved
easybuild/tools/options.py Outdated Show resolved Hide resolved
Format changes

Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
lexming and others added 14 commits September 12, 2024 16:25
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
…mmit

Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Copy link
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel
Copy link
Member

boegel commented Sep 18, 2024

Thanks a lot for all the effort on this @dagonzalezfo and @lexming!

@boegel boegel merged commit a2550eb into easybuilders:5.0.x Sep 18, 2024
36 checks passed
@boegel boegel changed the title More granular exit codes use more granular exit codes when EasyBuildError is raised Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change EasyBuild-5.0-blocker Blocker for EasyBuild 5.0 EasyBuild-5.0 EasyBuild 5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants