django-apscheduler icon indicating copy to clipboard operation
django-apscheduler copied to clipboard

Feature: simplified closing of old db connections

Open HeyHugo opened this issue 2 years ago • 4 comments

I'm using a customised BlockingScheduler class to ensure db connections are closed before/after running tasks.

This simplifies things for me in two ways:

  • I don't have to add the close_old_connections decorator to my tasks
  • I don't have to mock the close_old_connections decorator otherwise breaking my tests by prematurely closing the db connection my test session is using.

It looks like this

import functools
from apscheduler.schedulers.blocking import BlockingScheduler
from django import db

class BlockingDBScheduler(BlockingScheduler):
    def add_job(self, func, *args, **kwargs):
        @functools.wraps(func)
        def wrapper():
            db.close_old_connections()
            try:
                result = func(*args, **kwargs)
            finally:
                db.close_old_connections()
            return result

        return super().add_job(wrapper, *args, **kwargs)

If you'd think this class would fit as a part of the package I could make a PR, or if you think it'd be a nice addition to the docs as an alternative to the @close_old_connections-decorator I look at that as well.

HeyHugo avatar Jul 16 '22 19:07 HeyHugo

Would this achieve the same result and perhaps be easier to understand (untested)?

import functools
from apscheduler.schedulers.blocking import BlockingScheduler
from django import db

class BlockingDBScheduler(BlockingScheduler):
    @util.close_old_connections
    def add_job(self, func, *args, **kwargs):
        # Edited to swap `wrapper` with `func` as per the correction pointed out by @HeyHugo in the comments below.
        return super().add_job(func, *args, **kwargs)

jcass77 avatar Jul 22 '22 13:07 jcass77

@jcass77 Aha nice yes that works (just swap wrapper arg to func)

HeyHugo avatar Jul 22 '22 17:07 HeyHugo

Ok. I think this approach could be handy for folks who only intend to schedule jobs that always read or mutate the database.

The drawback is that if you use something like BlockingDBScheduler for anything else then those extra calls to db.close_old_connections() are essentially superfluous and just unnecessary overhead. That feels like an unreasonable tradeoff for the sake of convenience and syntactical sugar, and maybe not something that we want as the default behaviour for new users.

I'm happy to leave this here for others to find and use in their projects if their use case is narrowly defined along the lines discussed above though.

Thanks for pointing this out and providing a coding example!

jcass77 avatar Jul 23 '22 04:07 jcass77

Ok that's fair and I agree.

I have not yet had a case with a scheduled task without db interaction, but I understand there could be such use cases where state lives elsewhere.

HeyHugo avatar Jul 23 '22 09:07 HeyHugo