Repository navigation
Improve PR Labeler to scale well. - #389
Conversation
Preview EnvironmentA preview environment can be spun up on demand for this PR.
|
github-actions
Bot
added
github_actions
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
github-actions
Bot
removed
the
needs-triage
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14e4c8a703
ℹ️ 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".
subham-gp
left a comment
subham-gp
left a comment
There was a problem hiding this comment.
HI @akshitpatel1732
Everything looks good to merge once the Codex suggestions are resolved. Could you take a quick look and address those? Thanks!
github-actions
Bot
added
the
needs-review
The previous fix traded one scaling problem for another - solved the backfill's rate-limit crash, but reintroduced the exact same "fetch too much on every call" pattern on the real-time path, which is actually hit far more often (every push/open/reopen on every PR, forever) than a one-off backfill ever is. Fix: dropped the module-level cache entirely and replaced it with GitHub's server-side creator filter on the Issues API - scopes the query to just the one author being checked (not the whole repo), and stops paginating the instant it's confirmed the answer (as soon as a second PR by that author is seen). This is cheap and correct in both contexts without needing to know or care which caller invoked it - no more real-time-vs-backfill branching logic to reason about, which was really the root of the problem: two callers with very different access patterns sharing one cache that only made sense for one of them.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
github-actions
Bot
added
size/M
|
Hi @subham-gp, thanks for the review. I have resolved the issue highlighted by Codex and have also updated my workflows to ensure they are in line with the new Zizmor check. I have verified that failing workflow checks are stemming from the code beyond the scope of this PR and is solely because of the stale, unrelated code in the branch. This will not affect main when merged. Could you please have a look at this PR soon? Thanks. |
|
@codex review |
github-actions
Bot
removed
the
needs-review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaf72d05fd
ℹ️ 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".
github-actions
Bot
added
the
needs-review
github-actions
Bot
removed
the
needs-review
3 participants
Summary
Improves the PR Labeler by:
Type of Change
Affected Components
/backend-api/frontend/engine(collectors / policies)/security/infrastructure/.github/workflows/docsMotivation
The first backfill run for the PR Labeler failed partway because of the Search API's strict rate limits. Although this did not break anything and rerunning the backfill workflow gets the job done, it's better to solve this issue altogether by switching to pagination. Moreover, the workflow failed instead of continuing after the rate-limit reset because any error made the whole job fail.
Testing Done
Security Considerations
No security impact.
Breaking Changes
Rollback Plan
Checklist
Screenshots