tortoise / tortoise-orm

Familiar asyncio ORM for python, built with relations in mind
https://tortoise.github.io
Apache License 2.0
4.37k stars 355 forks source link

Feature: expose `timezone` and `use_tz` params in RegisterTortoise class #1632

Closed Abeautifulsnow closed 1 week ago

Abeautifulsnow commented 1 month ago

Is your feature request related to a problem? Please describe. I use mysql in my project and the I use db_url and RegisterTortoise to initialize the database connection. And then when I execute create on my Model, the new record is inserted into database, while the update_time is 8 hours behind the current time. That means that I need to set my timezone to Asia/Shanghai and use_tz to False cause I am in East 8 TimeZone.

But unfortunately I don't find the appropriate way to do that. I try to add timezone in db_url string, like this: mysql://root:123456@192.100.30.102:3306/energy_dispatch?charset=utf8&timezone=Asia/Shanghai, and the code is reported an error:

File "asyncmy/pool.pyx", line 216, in _create_pool                                                                                                                         
File "asyncmy/pool.pyx", line 217, in asyncmy.pool._create_pool                                                                                                            
File "asyncmy/pool.pyx", line 145, in fill_free_pool                                                                                                                       
File "asyncmy/connection.pyx", line 1278, in asyncmy.connection.connect                                                                                                  
TypeError: connect() got an unexpected keyword argument 'timezone'

I dig into the source code, and I do some changes here:

async def init_orm(self) -> None:  # pylint: disable=W0612
    config, config_file = self.config, self.config_file
    db_url, modules = self.db_url, self.modules
    await Tortoise.init(
        config=config,
        config_file=config_file,
        db_url=db_url,
        modules=modules,
        use_tz=False,    # --------------- here
        timezone="Asia/Shanghai",    # --------------------here
    )
    logger.info("Tortoise-ORM started, %s, %s", connections._get_storage(), Tortoise.apps)
    if self.generate_schemas:
        logger.info("Tortoise-ORM generating schema")
        await Tortoise.generate_schemas()

after that, I tried again and the time in db is correct now.

image

Describe the solution you'd like

I'm not sure this is a good choice to expose timezone and use_tz params to RegisterTortoise class.

class RegisterTortoise(AbstractAsyncContextManager):
    def __init__(
        self,
        app: FastAPI,
        config: Optional[dict] = None,
        config_file: Optional[str] = None,
        db_url: Optional[str] = None,
        modules: Optional[Dict[str, Iterable[Union[str, ModuleType]]]] = None,
        generate_schemas: bool = False,
        add_exception_handlers: bool = False,
        use_tz: bool = False,
        timezone: str = "UTC",
    ) -> None:
        self.app = app
        self.config = config
        self.config_file = config_file
        self.db_url = db_url
        self.modules = modules
        self.generate_schemas = generate_schemas
        self.use_tz = use_tz  # ------------here
        self.timezone = timezone  # ------------here

        if add_exception_handlers:

            @app.exception_handler(DoesNotExist)
            async def doesnotexist_exception_handler(request: "Request", exc: DoesNotExist):
                return JSONResponse(status_code=404, content={"detail": str(exc)})

            @app.exception_handler(IntegrityError)
            async def integrityerror_exception_handler(request: "Request", exc: IntegrityError):
                return JSONResponse(
                    status_code=422,
                    content={"detail": [{"loc": [], "msg": str(exc), "type": "IntegrityError"}]},
                )

    async def init_orm(self) -> None:  # pylint: disable=W0612
        config, config_file = self.config, self.config_file
        db_url, modules = self.db_url, self.modules
        await Tortoise.init(
            config=config,
            config_file=config_file,
            db_url=db_url,
            modules=modules,
            use_tz=self.use_tz,  # ------------here
            timezone=self.timezone,  # ------------here
        )
        logger.info("Tortoise-ORM started, %s, %s", connections._get_storage(), Tortoise.apps)
        if self.generate_schemas:
            logger.info("Tortoise-ORM generating schema")
            await Tortoise.generate_schemas()

    @staticmethod
    async def close_orm() -> None:  # pylint: disable=W0612
        await connections.close_all()
        logger.info("Tortoise-ORM shutdown")

    def __call__(self, *args, **kwargs) -> "RegisterTortoise":
        return self

    async def __aenter__(self) -> "RegisterTortoise":
        await self.init_orm()
        return self

    async def __aexit__(self, *args, **kw):
        await self.close_orm()

and I surely want to know If the db_url and config and config_file is identical each other?

if int(bool(config) + bool(config_file) + bool(db_url)) != 1:
        raise ConfigurationError(
            'You should init either from "config", "config_file" or "db_url"'
        )

If I select db_url to initialize my connection and how to config the timezone and use_tz and others, etc. I'd be appreciate if anyone could help clarify these confusions.

Abeautifulsnow commented 2 weeks ago

Can someone answer this question? Or point out if there is something unreasonable?

abondar commented 2 weeks ago

Hi!

I see no problem with exposing timezone params in RegisterTortoise Not sure why it wasn't added in the first place, probably contributor just forgot about it

You can contribute by making PR with this change

and I surely want to know If the db_url and config and config_file is identical each other?

Don't really understand issue here, this code just makes sure that you use only one of init methods, as we wouldn't know how to parse it if several are provided at the same time

Abeautifulsnow commented 2 weeks ago

Don't really understand issue here, this code just makes sure that you use only one of init methods, as we wouldn't know how to parse it if several are provided at the same time

My bad, I just don't know why adding timezone to db_url is causing an error, and I'm confused about whether all three are the same.

You can contribute by making PR with this change

Alright, I will create a new pull request for adding time_zone and use_tz in RegisterTortoise.

waketzheng commented 1 week ago

Currently, you can use the config argument:

@asynccontextmanager
async def lifespan(app: FastAPI):
    async with RegisterTortoise(
        app,
        config={
            "connections": {"default": DB_URL},
            "apps": {"models": {"models": ["models"]}},
            "use_tz": True,
            "timezone": "Asia/Shanghai",
            "generate_schemas": True,
        },
    ):
        yield
Abeautifulsnow commented 1 week ago

The feature has been extended in pr #1649