Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Default to adding idempotecy-key to audit-logs/events if not present, added in retryability logic to endpoints #492

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

Open
swaroopAkkineniWorkos wants to merge 27 commits into main
base: main
Choose a base branch
Loading
from ENT-3983-python-idempotency-retry

Conversation

@swaroopAkkineniWorkos
Copy link

@swaroopAkkineniWorkos swaroopAkkineniWorkos commented Nov 11, 2025
edited
Loading

Description

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

greptile-apps[bot] reacted with thumbs up emoji
Copy link

linear bot commented Nov 11, 2025

Copy link
Author

/review

@swaroopAkkineniWorkos swaroopAkkineniWorkos changed the title (削除) Ent 3983 python idempotency retry (削除ここまで) (追記) Default to adding idempotecy-key to audit-logs/events if not present, added in retryability logic to endpoints (追記ここまで) Nov 11, 2025
Copy link
Contributor

greptile-apps bot commented Nov 11, 2025
edited
Loading

Greptile Summary

  • Auto-generates UUID v4-based idempotency keys with workos-python- prefix for audit log events when not provided
  • Implements retry logic with exponential backoff for status codes 408, 500, 502, 504, network errors, and timeouts (max 3 retries, opt-in via retry_config parameter)
  • Changes create_event return type from None to AuditLogEventResponse with success field

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk.
  • The implementation is well-tested with comprehensive test coverage for both sync and async retry logic, follows existing patterns in the codebase, maintains backward compatibility by defaulting retry to opt-in (None by default), and the idempotency key auto-generation prevents duplicate events while allowing user override.
  • No files require special attention.

Important Files Changed

Filename Overview
workos/audit_logs.py Modified create_event to auto-generate idempotency keys with workos-python- prefix and enabled retry logic with default RetryConfig()
workos/utils/_base_http_client.py Added RetryConfig dataclass and helper methods _is_retryable_error, _is_retryable_exception, and _get_backoff_delay for retry functionality
workos/utils/http_client.py Implemented retry logic with exponential backoff in both SyncHTTPClient and AsyncHTTPClient.request() methods

Sequence Diagram

sequenceDiagram
 participant User
 participant AuditLogs
 participant HTTPClient
 participant API
 User->>AuditLogs: "create_event(org_id, event)"
 AuditLogs->>AuditLogs: "Generate idempotency key if not provided"
 AuditLogs->>HTTPClient: "request with retry configuration"
 
 loop Retry up to 3 times
 HTTPClient->>API: "POST audit logs events"
 alt Status 200
 API-->>HTTPClient: "Response body"
 HTTPClient-->>AuditLogs: "Return JSON"
 else Status 408, 500, 502, or 504
 API-->>HTTPClient: "Error response"
 HTTPClient->>HTTPClient: "Wait with exponential backoff"
 else Network or Timeout Error
 HTTPClient->>HTTPClient: "Wait with exponential backoff"
 end
 end
 
 AuditLogs->>AuditLogs: "Parse to AuditLogEventResponse"
 AuditLogs-->>User: "Return response object"
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

11 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@nave91 nave91 left a comment

Choose a reason for hiding this comment

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

The changes are perfect!

I found an issue before with this sdk: Can we return the response object when creating the audit log event? Especially for cases where schema validation fails for an event, the caller has no way of knowing that.

headers["idempotency-key"] = idempotency_key

# Enable retries for audit log event creation with default retryConfig
self._http_client.request(
Copy link

Choose a reason for hiding this comment

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

Can we return this response object back to the user?

I think we should also add some test cases when the schema validation fails.

Copy link
Author

@swaroopAkkineniWorkos swaroopAkkineniWorkos Nov 17, 2025

Choose a reason for hiding this comment

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

@nave91 updated to return the response back and added some validations

headers["idempotency-key"] = idempotency_key
# Auto-generate UUID v4 if not provided
if idempotency_key is None:
idempotency_key = str(uuid.uuid4())
Copy link

Choose a reason for hiding this comment

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

Same question - should we prefix with a string so we can differentiate sdk generated key vs customer?



@dataclass
class RetryConfig:
Copy link

Choose a reason for hiding this comment

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

💯 for using dataclasses, this keeps the retry config abstracted away 👌

Copy link
Author

@greptile re-review please

greptile-apps[bot] reacted with thumbs up emoji

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gcarvelli gcarvelli Awaiting requested review from gcarvelli gcarvelli is a code owner automatically assigned from workos/python

@jonatascastro12 jonatascastro12 Awaiting requested review from jonatascastro12 jonatascastro12 was automatically assigned from workos/enterprise

@csrbarber csrbarber Awaiting requested review from csrbarber csrbarber was automatically assigned from workos/enterprise

@ajworkos ajworkos Awaiting requested review from ajworkos ajworkos was automatically assigned from workos/enterprise

@katie-west katie-west Awaiting requested review from katie-west katie-west was automatically assigned from workos/enterprise

@nave91 nave91 Awaiting requested review from nave91

1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

enhancement New feature or request

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /