jcass77 / django-apscheduler

APScheduler for Django
MIT License
669 stars 97 forks source link

Close old DB connections allow to recover from DB errors on each iteration #147

Closed bluetech closed 3 years ago

bluetech commented 3 years ago

This is a proposed fix for #145. Please see the code comment for an explanation.

The added call can cause a connection to close, so it is somewhat assumed that get_due_jobs() is run from the scheduler loop in a context which "owns" a connection (e.g. a management command or dedicated thread) and not a shared/request context.

The close_old_connections API itself is not documented but has existed for a long time and I doubt it will disappear without notice.

The test is a bit hacky but it's what I managed with sqlite.

codecov[bot] commented 3 years ago

Codecov Report

Merging #147 (aea86a0) into develop (c3e9d24) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #147   +/-   ##
========================================
  Coverage    97.85%   97.85%           
========================================
  Files            6        6           
  Lines          326      327    +1     
========================================
+ Hits           319      320    +1     
  Misses           7        7           
Flag Coverage Δ
unittests 97.85% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
django_apscheduler/jobstores.py 98.67% <100.00%> (+<0.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c3e9d24...aea86a0. Read the comment docs.

jcass77 commented 3 years ago

Thanks for the PR. I think this is a hard problem to solve and I am not entirely sure how best to go about it.

Please see #148 for an alternative approach that attempts to cover more cases and avoid some of the many pitfalls and side effects. I would be grateful if you could try it out and report your findings there.