Skip to content

Conversation

@roggervalf
Copy link
Collaborator

Why

  1. Why is this change necessary? When consuming a deferred failure, rate limit is increased when it's not needed in this case. As we are not processing but failing only

How

Enter the implementation details here.

Additional Notes (Optional)

Any extra info here.

Copilot AI review requested due to automatic review settings July 21, 2025 05:17

This comment was marked as outdated.

@roggervalf roggervalf force-pushed the fix-defa-rate-limit branch 3 times, most recently from b370266 to dd1be4e Compare July 31, 2025 07:24
@roggervalf roggervalf requested review from Copilot and manast July 31, 2025 07:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a rate limiting issue where the rate limit counter was incorrectly being incremented when processing deferred failures. The change ensures that jobs with deferred failures bypass rate limiting since they are failing rather than being actively processed.

  • Adds logic to check for deferred failures before applying rate limiting
  • Introduces a helper function to extract deferred failure information from job attributes
  • Adds comprehensive test coverage for the parent-child job failure scenario

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/commands/includes/prepareJobForProcessing.lua Implements the core fix by checking for deferred failures before incrementing rate limit counter
tests/test_rate_limiter.ts Adds test case to verify rate limit counter is not incremented when consuming deferred failures
src/commands/getRateLimitTtl-1.lua Adds TODO comment about inconsistent return values

@roggervalf roggervalf force-pushed the fix-defa-rate-limit branch from dd1be4e to 6acfff6 Compare July 31, 2025 14:47
@roggervalf roggervalf force-pushed the fix-defa-rate-limit branch from 6acfff6 to e3a1782 Compare August 15, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants