Skip to content

implement exponential backoff in download_file + allow max. 6 attempts to download PR diff in fetch_files_from_pr and fetch_files_from_commit #4870

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

Merged
merged 7 commits into from
May 14, 2025

Conversation

boegel
Copy link
Member

@boegel boegel commented May 7, 2025

workaround/fix for #4869

@boegel boegel added this to the 5.0.1 milestone May 7, 2025
@boegel boegel added the bug fix label May 7, 2025
@boegel boegel changed the title implement exponential backoff in download_file + allow max. 8 attempts to download PR diff in fetch_files_from_pr implement exponential backoff in download_file + allow max. 8 attempts to download PR diff in fetch_files_from_pr May 7, 2025
@boegel
Copy link
Member Author

boegel commented May 8, 2025

I tested this with an "extreme" case, and it seems to hold up well:

$ eb --debug --from-pr 22698,22852,22827 --disable-cleanup-tmpdir -M --include-easyblocks-from-pr 3707
== Temporary log file in case of crash /tmp/eb-o9bahsfx/easybuild-zumur0uf.log
== Waiting for 20 seconds before trying download of https://github.com/easybuilders/easybuild-easyblocks/pull/3707.diff again...
== easyblock mathematica.py included from PR #3707
== Waiting for 20 seconds before trying download of https://github.com/easybuilders/easybuild-easyconfigs/pull/22698.diff again...
== Waiting for 40 seconds before trying download of https://github.com/easybuilders/easybuild-easyconfigs/pull/22698.diff again...
== Waiting for 20 seconds before trying download of https://github.com/easybuilders/easybuild-easyconfigs/pull/22852.diff again...
== Waiting for 40 seconds before trying download of https://github.com/easybuilders/easybuild-easyconfigs/pull/22852.diff again...
== Waiting for 20 seconds before trying download of https://github.com/easybuilders/easybuild-easyconfigs/pull/22827.diff again...
== Waiting for 40 seconds before trying download of https://github.com/easybuilders/easybuild-easyconfigs/pull/22827.diff again...

143 out of 146 required modules missing:

The printed messages I added for debugging purposes, but it's probably wise to actually have those, so I'll update the PR accordingly.

The first wait time is also not the intended 10 seconds, it straight goes to 2*10, need to fix that too.

@sassy-crick
Copy link

Just to add: I tried eb --copy-ec --from-pr 22824 which fails for the reasons mentioned above but with the fixes it works well. Also, I tried two different IP addresses, home and work, and I had the same issue. Thus, the IP address where you are coming from does not seem to make a difference. At home I got the full GitHub integration setup, and that did not make a difference too.

@smoors
Copy link
Contributor

smoors commented May 9, 2025

works well for me too

lexming
lexming previously approved these changes May 14, 2025
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

@boegel boegel changed the title implement exponential backoff in download_file + allow max. 8 attempts to download PR diff in fetch_files_from_pr implement exponential backoff in download_file + allow max. 6 attempts to download PR diff in fetch_files_from_pr May 14, 2025
@boegel boegel changed the title implement exponential backoff in download_file + allow max. 6 attempts to download PR diff in fetch_files_from_pr implement exponential backoff in download_file + allow max. 6 attempts to download PR diff in fetch_files_from_pr and fetch_files_from_commit May 14, 2025
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 2

@lexming lexming enabled auto-merge May 14, 2025 16:25
@lexming lexming merged commit 250d36a into easybuilders:develop May 14, 2025
37 checks passed
@boegel boegel deleted the from_pr_retries branch May 14, 2025 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants