-
Notifications
You must be signed in to change notification settings - Fork 33
Add auto-idempotency keys and retry logic for audit log events #405
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
- Add max_retries and auto_idempotency_keys config options - Implement exponential backoff with jitter for retryable errors - Auto-generate UUID v4 idempotency keys when not provided - Retry on 5xx, 429, and network timeout errors - Add retryable? method to WorkOSError - Add comprehensive test coverage for retry and idempotency features
- Remove auto_idempotency_keys config flag - Always auto-generate idempotency keys when not provided - Remove related test for disabled flag
- Set max_retries=3 in test setup since default is now 0 - Add after hook to reset config between tests - Fix successful retry test to properly mock response headers - Remove redundant manual reset calls in individual tests
- Add Metrics/PerceivedComplexity to existing rubocop:disable comment - Remove trailing whitespace in client_retry_spec.rb
|
|
||
| def initialize | ||
| @timeout = 60 | ||
| @max_retries = 0 |
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.
default to 0 retries
|
/review |
|
@greptile review plz |
Greptile Summary
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AuditLogs
participant Client
participant API
User->>AuditLogs: create_event(organization, event, idempotency_key)
AuditLogs->>AuditLogs: Generate UUID if idempotency_key is nil
AuditLogs->>Client: execute_request(request, retries: audit_log_max_retries)
Client->>API: POST /audit_logs/events with Idempotency-Key
alt Request succeeds (2xx)
API-->>Client: 200/201 response
Client-->>AuditLogs: response
AuditLogs-->>User: response
else Retryable error (5xx, 408, 429)
API-->>Client: 500/503/408/429 response
Client->>Client: Check attempt < retries
Client->>Client: Calculate backoff delay
Client->>Client: Sleep for delay
Client->>Client: Increment attempt counter
Client->>API: Retry POST with same Idempotency-Key
alt Retry succeeds
API-->>Client: 200/201 response
Client-->>AuditLogs: response
AuditLogs-->>User: response
else Max retries exceeded
API-->>Client: Still failing
Client->>Client: Raise error (APIError/TimeoutError)
Client-->>User: Error
end
else Non-retryable error (4xx)
API-->>Client: 400/401/403/404 response
Client->>Client: Raise error immediately
Client-->>User: Error
end
|
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.
7 files reviewed, 3 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
|
@greptile re-review plz |
|
@greptile re-review plz |
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.
7 files reviewed, 3 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
|
@greptile re-review plz |
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.
7 files reviewed, 2 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
|
@greptile re-review plz |
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.
7 files reviewed, no comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
|
@greptile re-review plz |
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.
6 files reviewed, no comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
Updating python library sdk to always generating an idempotencyKey if not present when POSTing audit-logs events.
Also updated to add retryability to endpoints. I set it up to not be enabled by default