Skip to content
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

Dump SeaORM Dependency from SeaSchema's Migrator #57

Closed
billy1624 opened this issue Mar 27, 2022 · 7 comments · Fixed by SeaQL/sea-orm#666
Closed

Dump SeaORM Dependency from SeaSchema's Migrator #57

billy1624 opened this issue Mar 27, 2022 · 7 comments · Fixed by SeaQL/sea-orm#666
Assignees

Comments

@billy1624
Copy link
Member

Motivation

A dependency tree of a typical SeaORM application looks like this. Which SeaORM is depends by the application and the underlying SeaSchema migrator. This is hard to maintain and difficult to release new version of SeaORM.

axum-graphql v0.1.0 (sea-orm/examples/axum-graphql_example)
├── entity v0.1.0 (sea-orm/examples/axum-graphql_example/entity)
│   ├── sea-orm v0.7.1
│   │   ├── sea-orm-macros v0.7.0 (proc-macro)
│   │   ├── sea-query v0.23.0
│   │   │   ├── sea-query-derive v0.2.0 (proc-macro)
│   │   ├── sea-strum v0.23.0
│   │   │   └── sea-strum_macros v0.23.0 (proc-macro)
├── migration v0.1.0 (sea-orm/examples/axum-graphql_example/migration)
│   ├── entity v0.1.0 (sea-orm/examples/axum-graphql_example/entity) (*)
│   └── sea-schema v0.7.1
│       ├── sea-orm v0.7.1 (*)
│       ├── sea-query v0.22.0
│       │   └── sea-query-derive v0.2.0 (proc-macro) (*)
│       ├── sea-schema-derive v0.1.0 (proc-macro)

Solution

We could get rid of SeaORM dependency on SeaSchema's migrator, and manage the connection to various db backend with sqlx::Any. sqxl::Pool::any_kind method can be used to identify the db backend of current connection.

@billy1624 billy1624 self-assigned this Mar 27, 2022
@billy1624
Copy link
Member Author

@tyt2y3 thoughts?

@tyt2y3
Copy link
Member

tyt2y3 commented Mar 27, 2022

If we remove the dependency all-together, how do we expose a proper connection inside the migration script?

let db = manager.get_connection();
cake::ActiveModel {
name: Set("Cheesecake".to_owned()),
..Default::default()
}
.insert(db)

@billy1624
Copy link
Member Author

Opppps. You're right!

Then, we should split sea-orm structs and enums that depends on sqlx into a new crate maybe called sea-connection. While leaving connection related traits inside sea-orm. This will also allows other connectors to be plugged into sea-orm.

@ikrivosheev
Copy link
Member

Opppps. You're right!

Then, we should split sea-orm structs and enums that depends on sqlx into a new crate maybe called sea-connection. While leaving connection related traits inside sea-orm. This will also allows other connectors to be plugged into sea-orm.

If the solution is suitable, I can prepare PR.

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 6, 2022

Then, we should split sea-orm structs and enums that depends on sqlx into a new crate maybe called sea-connection. While leaving connection related traits inside sea-orm. This will also allows other connectors to be plugged into sea-orm.

I don't understand how that'd help the current situation. The culprit is that we have an inter-locked release procedure:

  1. release sea-orm
  2. release sea-schema depends on the newly released sea-orm
  3. release sea-orm-cli depends on the newly released sea-schema

Which is kind of wrong. Ideally, it should be:

  1. release sea-schema
  2. release sea-orm-cli
  3. release sea-orm

May be the best and simplest way is to move the sea-orm facing functionalities (migrator logic) into sea-orm-cli itself, and leave only the schema manager in sea-schema.

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 6, 2022

#40

@billy1624
Copy link
Member Author

May be the best and simplest way is to move the sea-orm facing functionalities (migrator logic) into sea-orm-cli itself, and leave only the schema manager in sea-schema.

For sea-schema, we could keep most of the migrator logic intact while introduce a new connection trait, MigratorConnection, in it. Then, implement MigratorConnection for sea_orm::DatabaseConnection. In this way, sea-schema doesn't depends on sea-orm thus breaking the deadlock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants