-
Notifications
You must be signed in to change notification settings - Fork 0
Replace bare Exception catches with specific exception types and add graceful shutdown #180
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: redreceipt <2659478+redreceipt@users.noreply.github.com>
Co-authored-by: redreceipt <2659478+redreceipt@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try: | ||
| completed = get_completed_issues_for_person(login, 7) | ||
| except Exception as e: | ||
| except (requests.RequestException, ValueError) as e: | ||
| logging.error(f"Failed to fetch completed issues for {login}: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catch Linear client errors when listing inactive engineers
The new (requests.RequestException, ValueError) catch in post_inactive_engineers no longer matches the errors produced by get_completed_issues_for_person, which uses the gql client over aiohttp and raises TransportQueryError/aiohttp.ClientError rather than requests.RequestException. Those failures will now bubble out of the scheduled job (the with_retries wrapper re-raises), stopping the scheduler loop instead of logging and continuing as before when Linear is unavailable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codex could you address this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
- Added specific exception imports for Linear GraphQL transport, aiohttp client, and OpenAI SDK to align scheduler error handling with actual client failures. jobs.pyL1-L14
- Updated
post_inactive_engineersto log and continue on Linear transport/client errors or bad data instead of letting scheduler jobs crash. jobs.pyL489-L503 - Ensured weekly changelog generation catches OpenAI errors alongside request/validation issues to preserve scheduler resilience. jobs.pyL680-L692
Testing
⚠️ Not run (not requested)
| except (requests.RequestException, ValueError) as e: | ||
| logging.error( | ||
| "Failed to generate changelog via function call. Error: %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenAI failures now crash weekly changelog job
post_weekly_changelog now only catches requests.RequestException/ValueError, but get_chat_function_call is implemented with the OpenAI SDK and will raise OpenAIError (or other SDK/network exceptions) instead. Any transient OpenAI outage will therefore propagate and terminate the scheduler instead of being logged and returning an empty changelog as before, reducing resilience of the weekly job.
Useful? React with 👍 / 👎.
Replaces overly broad
except Exceptionclauses that could mask unexpected errors, and adds signal handling for clean process termination.Exception Handling
_get_pr_diffs(): Catchrequests.RequestExceptionfor network failures when fetching PR diffspost_inactive_engineers(): Catch(requests.RequestException, ValueError)for API failures fetching completed issuespost_weekly_changelog(): Catch(requests.RequestException, ValueError)for OpenAI API call failuresSignal Handling
Added SIGINT/SIGTERM handlers to break the scheduler loop cleanly instead of running indefinitely:
Minor Refactor
Moved
support_slugs = get_support_slugs()before dependent variable declarations inpost_priority_bugs()to reduce scope.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.