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

More granular exit codes #4534

Open
wants to merge 22 commits into
base: 5.0.x
Choose a base branch
from

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
Copy link
Author

Dear @lexming , @boegel, @hvelab ,

I just accept all the changes proposed by Alex, do minimal style fixes and update the PR.

If it needs futher work, just let me know.

@boegel
Copy link
Member

boegel commented Aug 27, 2024

Some minor code style issues to fix here (@lexming?):

./easybuild/framework/easyconfig/tools.py:226:9: F821 undefined name 'print_error'
./easybuild/tools/build_log.py:122:1: E302 expected 2 blank lines, found 1

@dagonzalezfo dagonzalezfo changed the title More granular exit codes WIP:More granular exit codes Aug 30, 2024
@lexming lexming changed the title WIP:More granular exit codes More granular exit codes 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

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
Status: Breaking changes
Development

Successfully merging this pull request may close these issues.

None yet

5 participants