Tenant provisioning in the dataplane #2694
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements tenant provisioning in the dataplane for the Danswer project, introducing multi-tenancy support across various components.
- Added new
CreateTenantRequestmodel inbackend/ee/danswer/server/tenants/models.pyfor tenant creation - Implemented tenant creation endpoint in
backend/ee/danswer/server/tenants/api.pywith schema creation and Alembic migrations - Modified
backend/danswer/db/engine.pyto support multi-tenant database operations, including schema switching - Added
control_plane_depfunction inbackend/danswer/auth/users.pyfor JWT-based authentication in the control plane - Updated
backend/alembic/env.pyto support multi-tenant schema migrations
17 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings
| payload = jwt.decode(token, DATA_PLANE_SECRET, algorithms=["HS256"]) | ||
| if payload.get("scope") != "tenant:create": |
There was a problem hiding this comment.
style: Consider using a more specific algorithm like 'HS256' instead of the generic 'HS256'
backend/danswer/db/engine.py
Outdated
| # Set the search_path to the tenant's schema | ||
| session.execute(text(f'SET search_path = "{tenant_id}"')) |
There was a problem hiding this comment.
style: Use parameterized query to set search_path for better security
backend/danswer/db/engine.py
Outdated
| if not is_valid_schema_name(tenant_id): | ||
| raise HTTPException(status_code=400, detail="Invalid tenant ID") | ||
| # Set the search_path to the tenant's schema | ||
| await async_session.execute(text(f'SET search_path = "{tenant_id}"')) |
There was a problem hiding this comment.
style: Use parameterized query to set search_path for better security
| else: | ||
| logger.info(f"Schema already exists for tenant {tenant_id}") | ||
|
|
||
| run_alembic_migrations(tenant_id) |
There was a problem hiding this comment.
style: Consider wrapping this in a try-except block to handle migration failures
| except Exception as e: | ||
| logger.exception(f"Failed to create tenant {tenant_id}: {str(e)}") | ||
| raise HTTPException( | ||
| status_code=500, detail=f"Failed to create tenant: {str(e)}" | ||
| ) |
There was a problem hiding this comment.
style: Catch specific exceptions and handle them differently
| @@ -0,0 +1,6 @@ | |||
| from pydantic import BaseModel | |||
There was a problem hiding this comment.
style: Consider adding type hints for improved code readability and maintainability
| class CreateTenantRequest(BaseModel): | ||
| tenant_id: str | ||
| initial_admin_email: str |
There was a problem hiding this comment.
style: Consider adding field descriptions using Pydantic's Field for better documentation
| alembic_cfg.cmd_opts = SimpleNamespace() # type: ignore | ||
| alembic_cfg.cmd_opts.x = [f"schema={schema_name}"] # type: ignore |
There was a problem hiding this comment.
style: Type ignores used here. Consider adding proper type annotations or explaining why they're necessary.
| # Run migrations programmatically | ||
| command.upgrade(alembic_cfg, "head") | ||
|
|
||
| # Run migrations programmatically | ||
| logger.info( |
There was a problem hiding this comment.
style: Duplicate comment 'Run migrations programmatically'. Remove one instance.
pablonyx
force-pushed
the
tenant_provisioning_dataplane
branch
from
2543664 to
748d7e1
Compare
pablonyx
force-pushed
the
tenant_provisioning_dataplane
branch
from
6a7250d to
b579648
Compare
Description
[Provide a brief description of the changes in this PR]
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Accepted Risk
[Any know risks or failure modes to point out to reviewers]
Related Issue(s)
[If applicable, link to the issue(s) this PR addresses]
Checklist: