flask-sqlalchemy

flask-sqlalchemy copied to clipboard

I am using version 2.4.1. I am trying to set my poolclass to NullPool by passing that option into my engine options like so:

from sqlalchemy.pool import NullPool
SQLALCHEMY_ENGINE_OPTIONS = {"poolclass":NullPool}

That is working as expected and the poolclass is being set. However I am getting the following error when running:

TypeError: Invalid argument(s) 'pool_size' sent to create_engine(), using configuration AuroraMySQLDataAPIDialect/NullPool/Engine.  Please check that the keyword arguments are appropriate for this combination of components.

The pool_size needs to NOT be set in order to use NullPool. This seems to be happening because the pool_size is being set to 10. I did some digging and found it was being set in apply_driver_hacks:

        if sa_url.drivername.startswith('mysql'):
            sa_url.query.setdefault('charset', 'utf8')
            if sa_url.drivername != 'mysql+gaerdbms':
                options.setdefault('pool_size', 10)
                options.setdefault('pool_recycle', 7200)

If I comment out those last 3 lines and run my code, the pool size doesn't get set and everything works as expected.

But I would like to figure out a way to prevent the pool size from being set to 10 without having to fork the repo and make this change to do so. Is there any way to do this? I have tried passing in pool_size:0 and pool_size:None into my engine options but it is still getting set to 10 regardless.

FWIW I am using a custom dialect (https://github.com/chanzuckerberg/sqlalchemy-aurora-data-api), but the driver name starts with "mysql" so it's being caught by this logic regardless.

I agree - this looks like a bug. I've proposed a fix in the linked PR #805 In the short term (and providing the Aurora data API supports pooling), could you use a static pool instead:

https://docs.sqlalchemy.org/en/13/core/pooling.html?highlight=staticpool#sqlalchemy.pool.StaticPool

Hi, any updates regarding this? Like previous posters, I currently need to use the patch suggested by @olinger since there doesn't seem to be any better workaround available.

Hi, any updates regarding this? Like previous posters, I currently need to use the patch suggested by @olinger since there doesn't seem to be any better workaround available.

I've closed that PR - it got a bit messed up between a rebase, and some back and forth. Let me see if I can revisit this in the next week or so.

Thanks @CoburnJoe - much appreciated!

@olinger @bfaivre @JoohnSF I have a potential fix in my fork https://github.com/CoburnJoe/flask-sqlalchemy/tree/master/ Please can you try this out for your use-case and let me know if it's suitable? I'll move it into PR if so.

@CoburnJoe works for my use-case! 🙏

Yes, works for me too. Thanks!

Just ran into this issue as well and the fix in @CoburnJoe 's fork would work for my use-case as well.

Thank you for confirming this change @traviscook21 @JoohnSF @bfaivre I'll need some time to write the unit tests (looks like there are not any for the apply_driver_hacks function), but I'll get that done as soon as possible - I'd estimate 1-2 weeks (sorry for the delay!).

Fixed CoburnJoe fix for V3 Flask-SQLAlchemy. Applied fix for NullPool and StaticPool that has same error https://github.com/SixK/flask-sqlalchemy

Hi @CoburnJoe - any update on when we might see this merged and released?

@bfaivre so sorry for the delay - I'm really struggling with time at the moment. Perhaps @davidism can chip with regards to the unit tests - given we don't have tests for this part of the code to begin with...

Edit: chip in as in discuss not write!

Thanks @CoburnJoe - totally understand and appreciate everyone's efforts.

Cheers, Brian

Is there an open PR for this? If it's hard to write tests for, just note that down in the PR and be sure to write a good changelog so we know where to look later.

Thanks @davidism. Hi @CoburnJoe - just bumping this issue.

Cheers,

Brian

Thanks @davidism. Hi @CoburnJoe - just bumping this issue.

Cheers,

Brian

Sorry folks - I've been well and truly rubbish on this one. I have a pretty intense job writing Python, so still haven't caught up on this 👎

Would anyone else be willing to pick up change?