-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(messaging): use SES tenants #39129
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
Conversation
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.
1 file reviewed, 2 comments
| self.client.create_tenant_resource_association( | ||
| TenantName=tenant_name, | ||
| ResourceArn=f"arn:aws:ses:{settings.SES_REGION}:{settings.SES_ACCOUNT_ID}:identity/{domain}", | ||
| ) |
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.
logic: The tenant-resource association call lacks error handling, which could leave the system in an inconsistent state if tenant creation succeeds but association fails
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/messaging/backend/providers/ses.py
Line: 37:40
Comment:
**logic:** The tenant-resource association call lacks error handling, which could leave the system in an inconsistent state if tenant creation succeeds but association fails
How can I resolve this? If you propose a fix, please make it concise.| try: | ||
| tenant_name = f"team-{team_id}" | ||
| self.client.create_tenant(TenantName=tenant_name, Tags=[{"Key": "team_id", "Value": str(team_id)}]) |
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.
style: AWS tenant names have specific constraints (3-64 characters, alphanumeric and hyphens). Consider validating the generated tenant name meets these requirements
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/messaging/backend/providers/ses.py
Line: 29:31
Comment:
**style:** AWS tenant names have specific constraints (3-64 characters, alphanumeric and hyphens). Consider validating the generated tenant name meets these requirements
How can I resolve this? If you propose a fix, please make it concise.…/messaging-ses-multi-tenant
…PostHog/posthog into feat/messaging-ses-multi-tenant
…PostHog/posthog into feat/messaging-ses-multi-tenant
…/messaging-ses-multi-tenant
…PostHog/posthog into feat/messaging-ses-multi-tenant
This reverts commit 046a01f.
Problem
We're not using Tenants yet, a newer feature of SES that will give us better observability on customer-by-customer deliverability metrics
Changes
Adds a couple of calls to create a tenant for a team ID and associate new domains with it
How did you test this code?
TBD