schema: multi-tenancy

[sadath-12]

Summary

The pr allows the users to pass schema as tenant in headers if the users wants multi-tenancy into their application .
Also it removes Form() reading from /basic/user and uses request.json() to read from body as pydantic does not reset the pointers and causes stream consumed error

Test Plan

• PR has an associated issue: Feature: Multitenancy support  #1122
• make check passes
• make test shows 100% unit test coverage

Deployment Plan

[netlify]

✅ Deploy Preview for thriving-cassata-78ae72 canceled.

Name	Link
🔨 Latest commit	f046828
🔍 Latest deploy log	https://app.netlify.com/sites/thriving-cassata-78ae72/deploys/66bc5ef6a7e6cf00087f9240

[samredai]

Related to my other comment, I would just pass in the schema here. so:

session_manager = get_session_manager(schema=request.headers.get("schema")

[sadath-12]

I have updated the implementation to only route to different schema and remove creation of schema code as discussed with @samredai

[samredai]

@sadath-12 can you help me understand why you need to modify the settings instance directly like this? Also, since you're pulling schema from the "tenant" header, you don't need this settings property at all anymore, right?

[sadath-12]

Hi @samredai , this is to tell dj to switch to the schema when we hit register schema endpoint (created internally) so dj can run migrations on it on runtime or else it will have to be restarted

[samredai]

Thanks @sadath-12 I understand the intent here now. I don't think modifying the application settings is the right approach though. Instead we should parameterize the alembic migration scripts (I think I recall @shangyian proposing this). Then in your custom endpoint logic, you can programmatically set that schema parameter using the context.

[ockhamlabs]

Wondering if this is close to finalization? Anything we can do to help ?

[samredai]

@ockhamlabs sorry for the late reply here. Let's revisit this after this refactor for the query service goes in which should make this ultimately much easier to achieve. I'm planning to catch up with @sadath-12 soon, either during the community meeting or async.

[sadath-12]

(after talk with @samredai )
Note: Whenever we switch across schemas make sure u use existing connection pool

[samredai]

@sadath-12 super sorry for losing track of this. I think we're close to figuring this out--this PR is doing the right thing by parsing the tenant value from the request header, but let's save any of the changes in the datajunction-query directory or to the alembic/env.py file for a follow up PR. If you could update this PR to just include the changes to datajunction_server/config.py and datajunction_server/utils.py, we can iterate on that and get this PR merged pretty quickly.

[samredai]

@sadath-12 from our discussions I remember you telling me that you had to do this so that the alembic script would run properly when you init a new tenant's schema, but since this commit the alembic scripts are now configurable to target a specific schema. This means in any of your custom endpoints you could do something like this:

from alembic.config import Config
from alembic import command

...
SCHEMA = request.headers.get("tenant") # For example, "tenant1"
if SCHEMA:
    alembic_cfg = Config()
    alembic_cfg.set_main_option("script_location", "./alembic")
    alembic_cfg.cmd_opts = {"x": [f"uri=postgresql+psycopg://username:password@server:5432/{SCHEMA}"]}
    command.upgrade(alembic_cfg, "head")
...

Which means you don't need to add this customSchema to the settings class at all.