Add organization domains module #464
Conversation
ajworkos
force-pushed
the
add-organization-domains-module
branch
from
ad93796 to
b0c5e71
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR creates a dedicated organization_domains module to align the Python SDK architecture with the Go and Node.js SDKs. The change extracts organization domain functionality from the broader organizations module into its own specialized module, following the principle of separation of concerns.
The implementation includes:
- A new
OrganizationDomainsModulewith CRUD operations (get_organization_domain,create_organization_domain,verify_organization_domain,delete_organization_domain) - Both synchronous and asynchronous client implementations following established patterns
- Proper Protocol definition for type safety
- Updates to base client classes to expose the new module via an
organization_domainsproperty - Migration of the
OrganizationDomaintype fromworkos.types.organizationstoworkos.types.organization_domains - Comprehensive test coverage matching existing testing patterns
The refactoring maintains backward compatibility at the API level while reorganizing the internal module structure. All import paths have been updated throughout the codebase to reference the new module location. The new module follows the same lazy initialization pattern used by other modules, where the module instance is created only when first accessed.
Confidence score: 3/5
- This PR is generally safe to merge but has one incomplete refactoring that needs attention
- The main issue is inconsistent cleanup in the organizations module where
AsyncOrganizationsstill contains adelete_organization_domainmethod that should have been removed - The file
workos/organizations.pyneeds more attention due to incomplete method removal and potential breaking changes for existing users
17 files reviewed, 1 comment
tests/test_organization_domains.py
Outdated
| "created_at": datetime.datetime.now().isoformat(), | ||
| "updated_at": datetime.datetime.now().isoformat(), |
There was a problem hiding this comment.
style: Using dynamic timestamps with datetime.now().isoformat() in test fixtures can cause flakiness. Consider using fixed timestamps for consistent test behavior.
ajworkos
force-pushed
the
add-organization-domains-module
branch
from
b0c5e71 to
d54a40d
Compare
|
Regarding:
The |
mattgd
left a comment
mattgd
left a comment
There was a problem hiding this comment.
Left one minor comment, but otherwise looks good.
| def delete_organization_domain( | ||
| self, organization_domain_id: str | ||
| ) -> SyncOrAsync[None]: | ||
| """Deletes a single Organization Domain | ||
| Args: | ||
| organization_domain_id (str): Organization Domain unique identifier | ||
| Returns: | ||
| None | ||
| """ | ||
| ... |
There was a problem hiding this comment.
Noting here that this is non-breaking because it was never released.
| from .organization_common import * | ||
| from .organization_domain import * | ||
| from .organization import * | ||
| from workos.types.organization_domains import OrganizationDomain |
There was a problem hiding this comment.
Could we add a little note that this import alias can be removed in a future major version bump?
Description
Aligns our python SDK with out Go and Node SDKs by adding a separate organization_domains module
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.
https://github.com/workos/workos/pull/42288