feat: expose classmethod to build tortoise config by waketzheng · Pull Request #2162 · tortoise/tortoise-orm

waketzheng · 2026-03-31T11:54:18Z

Description

The init function of tortoise.context.Context is too long
Add some new classmethods to TortoiseConfig which is used for sharing config between contexts

Motivation and Context

This function has too many lines; shorten it.

How Has This Been Tested?

make ci

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added the changelog accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

codspeed-hq · 2026-03-31T11:57:05Z

Merging this PR will not alter performance

✅ 24 untouched benchmarks

_{Comparing waketzheng:feat-typed-config (9ea070b) with develop (f89618d)}

seladb · 2026-04-05T06:51:58Z

@waketzheng this PR has many changes that are not related to the PR title and description. Maybe you should break them into multiple PRs?

seladb · 2026-04-05T20:15:19Z

@waketzheng this PR still seems to have unrelated changes. We have this PR to add pre-commit, and this PR for Starlette v1 support. These changes can probably be removed from this PR

waketzheng · 2026-04-06T06:51:32Z

@waketzheng this PR still seems to have unrelated changes. We have this PR to add pre-commit, and this PR for Starlette v1 support. These changes can probably be removed from this PR

@seladb No need to do that. I expected this PR to be reviewed after the previous one merged, so I explicitly mentioned that it’s based on #2159. Actually, I refactored the code once when fixing mypy complaints after Starlette upgraded, and then separated it into three PRs.

seladb · 2026-04-06T07:07:16Z

@waketzheng this PR still seems to have unrelated changes. We have this PR to add pre-commit, and this PR for Starlette v1 support. These changes can probably be removed from this PR

@seladb No need to do that. I expected this PR to be reviewed after the previous one merged, so I explicitly mentioned that it’s based on #2159. Actually, I refactored the code once when fixing mypy complaints after Starlette upgraded, and then separated it into three PRs.

@waketzheng is there a reason to open these changes as stacked PRs? It looks like these are separate changes that can each have their own PR based on develop 🤔

waketzheng · 2026-04-06T07:27:50Z

Using stacked PRs avoids merge conflicts
I can prek run --all-files to check this PR (prek is a reimagined version of pre-commit, built in Rust.)
You can select the latest commit (instead of all commits) to compare the code between this PR and the previous one.

seladb · 2026-04-06T07:36:55Z

Using stacked PRs avoids merge conflicts

Yes, if the changes in the stacked PRs depend on each other, otherwise there shouldn't be merge conflicts. Here it looks like the actual commit related to this PR doesn't depend on the changes in the other PR. What am I missing here? 🤔

…typed-config

waketzheng · 2026-04-06T13:34:16Z

Yes, if the changes in the stacked PRs depend on each other, otherwise there shouldn't be merge conflicts. Here it looks like the actual commit related to this PR doesn't depend on the changes in the other PR. What am I missing here? 🤔

You forgot that I need the pre-commit config to enforce code style.

seladb · 2026-04-07T05:21:20Z

You forgot that I need the pre-commit config to enforce code style.

I'm not sure why pre-commit is needed for this PR, but anyway - the pre-commit PR was merged so this PR is simpler now 🙂

…typed-config

seladb

Please see one comment, otherwise LGTM

…typed-config

seladb · 2026-04-15T06:50:38Z

        )
+
+    @classmethod
+    def merge_args(


I'm not sure the method name is clear, merge_args seems to create a merged configuration from config, config_file, db_url and modules, but this is actually not the case.

Maybe generate_config could be a better name.

Moreover, maybe we need to normalize the classmethods in this class:

@classmethod def generate_config( cls, config: dict[str, Any] | Self | None = None, config_file: str | None = None, db_url: str | None = None, modules: dict[str, Iterable[str | ModuleType]] | None = None, ) -> Self: ... @classmethod def generate_config_from_db_url_and_modules(cls, db_url: str, modules: dict[str, Iterable[str | ModuleType]]) -> Self: ... @classmethod def generate_config_from_config_file(cls, config_file: str) -> Self: ...

Thanks for the suggestions. I’ve updated the naming:

To keep consistent with from_dict, I’ll use from_db_url_and_modules / from_config_file.

Since there’s already a parameter called config, I used resolve_args instead of generate_config to avoid confusion.

I like the from_* convention and it's consistent with the existing code. Maybe we can rename resolve_args to from_args? 🤔

Maybe we can rename resolve_args to from_args?

That is one solution. However, I strongly recommend using resolve_* instead of from_* for this function, because:

resolve_* checks whether arguments conflict, whereas from_* does not.

resolve indicates that this function will parse the arguments, not just load configuration from them.

I think from_* is clearer, but we can let @abondar decide...

seladb

LGTM

abondar

There is duplication of args in docstring
I think resolve_args is okay

We need to add tests regarding these new methods, even if simple ones

Otherwise - looks good to me, @waketzheng you can fix these issues and merge without seeking my explicit reapproval

abondar · 2026-04-30T14:27:52Z

+        Args:
+            config (dict[str, Any] | TortoiseConfig | None):
+            config_file (str | None): Path to a config YAML or JSON file.
+            db_url (str | None): Database URL for config generation.
+            modules (dict[str, Iterable[str | ModuleType]] | None): App modules for config generation.
+        Args:
+            config: A configuration dict or TortoiseConfig instance.
+            config_file: Path to config file.
+            db_url: Database URL for config generation.
+            modules: App modules for config generation.


args are duplicated

abondar previously approved these changes Apr 5, 2026

View reviewed changes

waketzheng dismissed abondar’s stale review via 41be834 April 5, 2026 18:48

waketzheng requested a review from abondar April 5, 2026 19:13

waketzheng added 2 commits April 6, 2026 14:55

feat: add pre commit config and fix issues

23467a9

Fix mypy complaint

e656ce9

feat: expose classmethod to build tortoise config

b1e1093

waketzheng force-pushed the feat-typed-config branch from 7e95e57 to b1e1093 Compare April 6, 2026 07:16

Merge branch 'develop' of github.com:tortoise/tortoise-orm into feat-…

1de8dfc

…typed-config

seladb reviewed Apr 7, 2026

View reviewed changes

Comment thread tortoise/config.py Outdated

Comment thread tortoise/config.py Outdated

waketzheng added 2 commits April 10, 2026 00:46

refactor: move config file parse logic to TortoiseConfig

d7d62d5

Merge branch 'develop' of github.com:tortoise/tortoise-orm into feat-…

0cf3b41

…typed-config

waketzheng requested a review from seladb April 9, 2026 17:09

seladb approved these changes Apr 12, 2026

View reviewed changes

Comment thread tortoise/config.py Outdated

waketzheng added 3 commits April 13, 2026 21:12

Merge branch 'develop' of github.com:tortoise/tortoise-orm into feat-…

c9e905a

…typed-config

Use Self

260de75

refactor: reduce duplicated

5f71748

waketzheng requested a review from seladb April 14, 2026 17:18

seladb reviewed Apr 15, 2026

View reviewed changes

waketzheng added 2 commits April 17, 2026 23:42

refactor: change function names and add docs

3b78e1e

refactor: remove else

5167053

waketzheng requested a review from seladb April 17, 2026 16:10

waketzheng added 2 commits April 24, 2026 19:37

refactor: reduce lines and indent

cb9f3f4

Move import to top and use Self instead of class name for type hints

9ea070b

seladb approved these changes Apr 25, 2026

View reviewed changes

abondar approved these changes Apr 30, 2026

View reviewed changes

waketzheng merged commit 9d0e53e into tortoise:develop May 1, 2026
25 checks passed

Conversation

waketzheng commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Checklist:

Uh oh!

codspeed-hq Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

seladb commented Apr 5, 2026

Uh oh!

seladb commented Apr 5, 2026

Uh oh!

waketzheng commented Apr 6, 2026

Uh oh!

seladb commented Apr 6, 2026

Uh oh!

waketzheng commented Apr 6, 2026

Uh oh!

seladb commented Apr 6, 2026

Uh oh!

waketzheng commented Apr 6, 2026

Uh oh!

seladb commented Apr 7, 2026

Uh oh!

Uh oh!

Uh oh!

seladb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

seladb Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

waketzheng Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

seladb Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

waketzheng Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

seladb Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

seladb left a comment

Choose a reason for hiding this comment

Uh oh!

abondar left a comment

Choose a reason for hiding this comment

Uh oh!

abondar Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

waketzheng commented Mar 31, 2026 •

edited

Loading

codspeed-hq Bot commented Mar 31, 2026 •

edited

Loading