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

Merged
lexming merged 7 commits intoeasybuilders:developfrom
boegel:from_pr_retries
May 14, 2025
Merged

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
lexming merged 7 commits intoeasybuilders:developfrom
boegel:from_pr_retries

Conversation

@boegel
Copy link
Copy Markdown
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 boegel mentioned this pull request May 7, 2025
Closed
Micket
Micket reviewed May 7, 2025
View reviewed changes
Comment thread easybuild/tools/filetools.py Outdated
@boegel
Copy link
Copy Markdown
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
Copy Markdown

sassy-crick commented May 8, 2025

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
Copy Markdown
Contributor

smoors commented May 9, 2025

works well for me too

lexming
lexming reviewed May 13, 2025
View reviewed changes
Comment thread easybuild/tools/github.py Outdated
lexming
lexming previously approved these changes May 14, 2025
View reviewed changes
Copy link
Copy Markdown
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
lexming
lexming approved these changes May 14, 2025
View reviewed changes
Copy link
Copy Markdown
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
This was referenced May 15, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Micket Micket Micket left review comments

@lexming lexming lexming approved these changes

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

5.1.0

Development

Successfully merging this pull request may close these issues.

5 participants

@boegel @sassy-crick @smoors @Micket @lexming