Make queryset switch database schema

[rorik302]

I suggest making it possible for a queryset to change database schemas. For example, Model.with_db_schema(schema_name).query.all(). This would be great for multi-tenant applications

[tarsil]

This is a great idea. Although the Database allows to have the connections and manage them as well, having this seemless integrated is a good idea. While I investigate how can this happen it would be nice to have a PR with a possible solution and here some possible use cases as example (codewise) too

[tarsil]

So as per my initial investigation, SQLAlchemy __table_args__ exists in SQLAlchemy ORM and Edgy and Saffier use the SQLAlchemy core, which means that it does not exist. During the creation of the tables using SQLAlchemy core, Edgy uses the internal Registry which does not specify a schema parameter as default which means its none (defaults to the public schema) which is ok also for migrations purposes.

Unless you specify the schema in the metadata (via Registry object, meaning, deciding which registry to use)m it will always use the default one, The good news is that Saffier and Edgy Registry already support the schema parameter, which means you can have multiple registries declared in your models and simply specify the schema parameter. Example:

registry = Registry(database=...., schema="my_schema")
registry_public = Registry(database=....)

This should also work out of the box for alembic since to run the migrations wou would need to use the Migrate object from each Edgy and Saffier with the proper dedicated registry to generate the migrations from.

Now we can try to simplify this process somehow but for now I hope this helps.

[tarsil]

@rorik302 I am currently testing something that so far looks good. Allowing the queries into different schemas and even different databases is the database object is provided.

I have some concerns though. How are you going to manage the migrations of multiple schemas? You can query but I'm actually curious about that.

Also, how is Edgy so far working for you?

[rorik302]

To handle migrations with alembic I initialize some sections, for example

alembic.ini
[shared]
script_location = .../shared
[tenant]
script_location = .../tenant

In each section env.py file has own certain logic. To create migrations just use alembic -n shared (or tenant) revision --autogenerate ... in cli. Similar for migrate

I'm not using Edgy for now. But it looks promising. Lack of work with database schemas is the only thing that stops me

[tarsil]

Lack of work with database schemas is the only thing that stops me

Would you mind elaborating a bit more on this? Any feedback is welcomed :). Do you mean, what you are asking right now in this discussion? If yes, then probably that will change soon :)

[rorik302]

Yes. If work with database schemas will be implemented I will use Edgy or Saffier. As I know none of orm except sqlalchemy can't work with schemas without "crutches". But sqlalchemy is complicated for me and not user friendly

[tarsil]

I agree. SQLA is very complex and can be not very user friendly. Hence Edgy and Saffier. The good news is that I'm currently testing something and might need some work.

This PR #15 already has the "using" and the "using_with_db"

So the first is what you initially thought. Example

User.query.using("my-schema").all() 

The second also allows to pass a different database object with the schema name. Example:

new_database = Database (...)
User.query.using_with_db(database=database, schema="my-schema")

@rorik302 Do you think you can fork edgy and checkout this branch #15 and see if this does what you need?

I haven't updated the docs because I'm running some other tests and analysis but the tests I ran, it was working but it would be nice to have this on your side to check as well?

[rorik302]

I wonder how you have so much time to do so much work )). Thank you. I will check the code later

[tarsil]

Well it's in my free time + I also use Saffier and Edgy and Esmerald with my clients :)

[rorik302]

Should migrations work at this stage?

[tarsil]

The migrations are something else. There is another branch that I’m working on. Something that will be similar to this https://django-tenants.readthedocs.io/en/latest/

since I was the one making this https://django-tenants-url.tarsild.io/ (the error is related to the CI which I will fix soon).

This is the reason why I asked you how would you manage your migration side. For Edgy will be tricky making one migration per tenant because you might have 3000 tomorrow and 50000 the day after (hence my other PR trying to create a contrib (not core) in Edgy that does something similar to the links shared above).

this is very powerful feature like you said that you don’t see anywhere so I might take some time to get there but eventually we will 🙂

[rorik302]

I suppose that when using edgy init, I can specify sections that will then be added to alembic.ini and create folders, such as migrations/section1, migrations/section2. Each folder has own logic. One section controls shared/public schema, second section controls tenant migrations

[tarsil]

The init generates the initial alembic configurations, yes.

[rorik302]

I think it can be done like this: edgy init creates migration folder with alembic.ini, env.py, etc. But all those files need to replace to migrations/shared (for example), except alembic.ini, where will be created migrations for models without schema in registry (in my case tenant models). Then need to do edgy init --section public. This command adds section to alembic.ini and creates migrations/public folder, where will be created migrations that contains schema "public" in registry.

[tarsil]

Ok, can you provide me with more concrete examples like folder structure and a file example?

[rorik302]

public env.py

from logging.config import fileConfig

from alembic import context
from sqlalchemy import engine_from_config, text
from sqlalchemy import pool

from apps.models import core_models
from core.config import settings
from core.models import core_metadata

config = context.config

if config.config_file_name is not None:
    fileConfig(config.config_file_name)

target_metadata = core_metadata

config.set_main_option("sqlalchemy.url", settings.sync_db_url)

_ = core_models


def run_migrations_online() -> None:
    connectable = engine_from_config(
        config.get_section(config.config_ini_section, {}),
        prefix="sqlalchemy.",
        poolclass=pool.NullPool,
    )

    def include_object(object_, name, type_, reflected, compare_to):
        if type_ == "foreign_key_constraint":
            return False

        return True

    def process_revision_directives(context_, revision, directives):
        if config.cmd_opts.autogenerate:
            script = directives[0]
            if script.upgrade_ops.is_empty():
                directives[:] = []

    with connectable.connect() as connection:
        connection.execute(text(f"set search_path to {settings.CORE_SCHEMA_NAME}"))
        connection.commit()
        connection.dialect.default_schema_name = settings.CORE_SCHEMA_NAME

        context.configure(
            connection=connection,
            target_metadata=target_metadata,
            process_revision_directives=process_revision_directives,
            include_object=include_object
        )

        with context.begin_transaction():
            context.run_migrations()


run_migrations_online()

tenant env.py

from logging.config import fileConfig

from alembic import context
from sqlalchemy import engine_from_config, text
from sqlalchemy import pool

from apps.models import tenant_models
from core.config import settings
from core.models import tenant_metadata

config = context.config

if config.config_file_name is not None:
    fileConfig(config.config_file_name)

target_metadata = tenant_metadata

config.set_main_option("sqlalchemy.url", settings.sync_db_url)

_ = tenant_models


def run_migrations_online() -> None:
    connectable = engine_from_config(
        config.get_section(config.config_ini_section, {}),
        prefix="sqlalchemy.",
        poolclass=pool.NullPool,
    )

    def include_object(object_, name, type_, reflected, compare_to):
        if type_ == "foreign_key_constraint":
            return False

        return True

    def process_revision_directives(context_, revision, directives):
        if config.cmd_opts.autogenerate:
            script = directives[0]
            if script.upgrade_ops.is_empty():
                directives[:] = []

    tenant_schema = context.attributes.get("schema")

    with connectable.connect() as connection:
        connection.execute(text(f"set search_path to {tenant_schema}"))
        connection.commit()
        connection.dialect.default_schema_name = tenant_schema

        context.configure(
            connection=connection,
            target_metadata=target_metadata,
            process_revision_directives=process_revision_directives,
            include_object=include_object
        )

        with context.begin_transaction():
            context.run_migrations()


run_migrations_online()

[rorik302]

folder structure:

└── migrations
    ├── alembic.ini
    └── shared
        └── versions
        ├── env.py
        ├── README
        └── script.py.mako
    └── tenant
        └── versions
        ├── env.py
        ├── README
        └── script.py.mako

alembic.ini
[shared]
....
[tenant]
...

[tarsil]

@rorik302 I think I'm understanding what you are sugesting. I need to have a deep look.

Btw I merged something into the PR I told you to have a look. Its cleaner and simpler but I also added a "contrib", which means its not the core, just a suggestion from edgy that is similar to https://github.com/django-tenants/django-tenants/ . Which is the approach you and I are talking.

See the way I did the tests -> https://github.com/tarsil/edgy/pull/15/files#diff-7a5a06491a50457971419953b7a6b736aaf34bf8a5053163514ab916e6214734

Proves that the schemas if generated via contrib, work as expected. Of course the Edgy core with the `Model.query.using(...)' its agnostic to the contrib which is the introduction of the support to multi tenancy.

The contrib is a nice to have in general :)

Still a lot to do

[rorik302]

I found an issue. Seems edgy uses public schema by default. When I define schema in Registry, edgy uses public schema. If I delete public schema and call create_all() I have asyncpg exception "no schema has been selected to create in..."

[tarsil]

So, no, that is not an issue. By default of no schema is specified, ok n the registry (Param schema), SQLAlchemy will use the default schema of each supported database. In case of postgres it's called public, in SQL server its called dbo and so on so forth.

The public is what you called before the "shared".

Unless I'm not understanding what you meant without an example.

Registry with a schema its like this

Registry(database=database, schema="another")

[tarsil]

Give me the example you used please

[rorik302]

database = Database("postgresql+asyncpg://postgres:postgres@localhost:5432/test_db")
test_registry = Registry(database=database, schema="test")


class BaseModel(edgy.Model):
    uuid: UUID = edgy.UUIDField(default=uuid4)
    created_at: datetime = edgy.DateTimeField(default=datetime.utcnow, auto_now_add=True)
    updated_at: datetime = edgy.DateTimeField(default=datetime.utcnow, auto_now=True)
    is_active: bool = edgy.BooleanField(default=True)

    class Meta:
        abstract = True


class Tenant(edgy.Model):
    schema_name: str = edgy.CharField(max_length=250)

    class Meta:
        registry = test_registry


class User(edgy.Model):
    email: str = edgy.EmailField(max_length=250)
    password: str = edgy.CharField(max_length=250)
    tenant: Tenant = edgy.ForeignKey(Tenant, related_name="users")

    class Meta:
        registry = test_registry

Then I call await test_registry.create_all() and get "sqlalchemy.exc.DBAPIError: (sqlalchemy.dialects.postgresql.asyncpg.Error) <class 'asyncpg.exceptions.InvalidSchemaNameError'>: no schema has been selected to create in"

If database have public schema then ok, new schema created. But if no public schema throw exception

[tarsil]

Oh, you are trying to use the contrib? For that (I have built the docs yet, still running tests) your tenant must:

1. Use the Tenant mixin from edgy.contrib.multi_tenancy.models import TenantMixin
2. Use the TenantRegistry and the TenantModel from edgy.contrib.multi_tenancy import TenantModel, TenantRegistry. Your models should inherit from the TenantModel (which is an extra added on the top of the core edgy.Model) and the same for the TenantRegistry
3. In the models you want specifically to be applied to tenants, in the Meta you must specifiy is_tenant = True

This is the contrib, a solution that Edgy can possibly provide for you but you ar free to manually create your own schemas in the database and with the using() query those.

Does this make sense?

Although this is still a work in progress because I have other scenarios (outside of contrib) to test.

[tarsil]

@rorik302 you actually found another small issue with that example you gave me. I thought you were referring to something else so I was also testing for that case and it was indeed pointing to the public. That fix is now pushed to the branch (you can pull it).

But what said, remains, if you are trying to use the contrib, you will need what I told you above anyway.

The good news about all of this is once this is close, it will be applied 100% to Saffier as well 👍🏼

[tarsil]

Another good news it's that will be the availablity of a set_tenant() that will make your queries global to the tenant. Meaning? If you do a set_tenant you won't need to use the using(). This can be particularly useful if you have a Middleware Somewhere in your application that sets the tenant before reaching the API and therefore queries only the tenant data.

It's now in development and should be in branch soon

[tarsil]

Ok, great progress. Now pushed to the repo but basically the using is now more effective and the set_tenant was also added. You should not have those problems you mentioned above @rorik302 anymore :)

[rorik302]

One more question. I have models:

class Tenant(BaseModelMixin, edgy.Model):
    schema_name: str = edgy.CharField(max_length=255, unique=True)

    class Meta:
        registry = core_registry


class User(BaseModelMixin, edgy.Model):
    email: EmailStr = edgy.EmailField(max_length=250, unique=True)
    password: str = edgy.CharField(max_length=250)
    tenant: Tenant = edgy.ForeignKey(Tenant, on_delete=edgy.CASCADE)

    class Meta:
        registry = core_registry

How can I get list of tenants with related users? await Tenant.query.all() returns only tenant fields

[tarsil]

Oh,

Do you mean all the tenants and all the users of each tenant?

Oh that is model logic.

I would add a function in the Tenant like this:

async get_tenant_users(self) -> List[User]:
    return await User.query.filter(tenant=self)

This should do it nicely.

The using is now working nicely right? Also the issue you had with the schema is now solved?

Btw, thank you for this collaboration. Helped a lot with the design. It's almost done and ready I believe.

BUT because you have a FK in the user to the tenant, you can actually use the related field 😎. In the FK, add like this:

tenant: Tenant = edgy.ForeignKey(Tenant, on_delete=edgy.CASCADE, related_name="users")

Because now you have a related name users inside the FK to the tenant, you can query directly like this:

await Tenant.query.filter(users__tenant=self)

Nice right? Have a look at the docs. It's covered there :)

https://edgy.tarsild.io/queries/related-name/

[rorik302]

Thank you. Awesome work.
While Edgy doesn't support different migration sections out the box yet I think I can use alembic manually and use Edgy in development

[tarsil]

Oh thank you! This type of development makes my life easier and I believe majority of the developers as well.

Edgy natively doesn't support those migrations yet. A good design approach needs to be thought and I have a few possible ideas. In the meantime I'm just making the docs and explanations for what it does support at the moment because this is a great release and then I can apply it also to Saffier.

[tarsil]

@rorik302 the branch is now merged into main and this is exciting!! You can get the main as well and continue. I wont yet release because I want to add some extra docs as a nice to give from Edgy.

So far, is edgy treating you well? Multi tenancy is coming! :D

[tarsil]

@rorik302 I'm happy to announce that the new release is now out!

https://edgy.tarsild.io/tenancy/edgy/
https://edgy.tarsild.io/tenancy/contrib/ -> If you want of course

https://edgy.tarsild.io/release-notes/

There is no native support for multi tenant migrations yet because that needs to be properly planned but for now the full multi tenancy support on Edgy is out. Thank you for your help on this and if you are using Saffier. Saffier 0.17.0 is also out also with support for multi tenancy in the same fashion 👍🏼

[tarsil]

I will be closing this discussion @rorik302 as I think we got what it was initially suggested working well but feel free to tell me otherwise.

Good news is that I might have found a simpler way to give support to migrations with Edgy and Saffier and I’m currently working in that as we speak and should be out soon.

Awesome collaboration with you @rorik302 . Thank you very much.

[rorik302]

Thank you for very good projects and fast feedback

[tarsil]

My pleasure @rorik302 . It was a great interaction and a fantastic feature came out of it that benefits Edgy and Saffier. I'm the one who should thank 🙂

The one you mentioned for Esmerald is also fixed and deployed.

I hope you find the tools pleasant to work with in general 🙂