fix: correct missing f-string and off-by-one retry guard#237
Open
Nicknamess96 wants to merge 1 commit intoentrius:testfrom
Open
fix: correct missing f-string and off-by-one retry guard#237Nicknamess96 wants to merge 1 commit intoentrius:testfrom
Nicknamess96 wants to merge 1 commit intoentrius:testfrom
Conversation
…ithub_api_tools.py
- Add missing f-prefix to log message in issues_competition() so
{len(active_issues)} is interpolated instead of printed literally
- Fix off-by-one in get_pull_request_file_changes() retry guards:
change `attempt < max_attempts` to `attempt < max_attempts - 1`
to avoid unnecessary sleep after the final failed attempt
- Update existing tests and add new tests verifying no sleep occurs
after the last retry attempt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bug fixes found via code review:
1. Missing f-string prefix in
forward.py(line 259)Bug: The log message in
issues_competition()uses a plain string instead of an f-string:'***** Issue Bounties Complete: processed {len(active_issues)} issues (no state changes) *****'This prints the literal text
{len(active_issues)}instead of the actual count.Fix: Added the
fprefix so the variable is interpolated correctly.2. Off-by-one in
get_pull_request_file_changes()retry guard (lines 279, 289)Bug: The condition
attempt < max_attemptsis alwaysTrueinside arange(max_attempts)loop, causing an unnecessarytime.sleep()(up to 20s) after the last failed attempt before the function returns. This does not match the pattern used inexecute_graphql_query()andget_github_graphql_query(), which both correctly useattempt < (max_attempts - 1).Fix: Changed to
attempt < max_attempts - 1in both the HTTP error andRequestExceptionbranches.Testing
test_gives_up_after_three_attempts: corrected sleep count assertion from 3 to 2test_exponential_backoff_timing: corrected expected delays from[5, 10, 20]to[5, 10]test_no_sleep_after_final_http_error: verifies no sleep after final HTTP errortest_no_sleep_after_final_connection_error: verifies no sleep after final connection timeoutType of Change
Checklist