search

jcass77 / django-apscheduler

APScheduler for Django
MIT License
669 stars 97 forks source link

catch all django db error at decorator retry_on_db_operational_error #151

Closed zmmfsj-z closed 3 years ago

zmmfsj-z commented 3 years ago

OperationalError seems not to cover all exceptions eg: I catched InterfaceError while django lose db connection.

codecov[bot] commented 3 years ago

Codecov Report

Merging #151 (dbbc3b0) into develop (2cc7e91) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #151   +/-   ##
========================================
  Coverage    98.05%   98.05%           
========================================
  Files            6        6           
  Lines          359      359           
========================================
  Hits           352      352           
  Misses           7        7
Flag Coverage Δ
unittests 98.05% <100.00%> (ø)

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

Impacted Files Coverage Δ
django_apscheduler/util.py 97.82% <100.00%> (ø)

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 2cc7e91...dbbc3b0. Read the comment docs.

jcass77 commented 3 years ago

Thanks for the PR. There are a number of subclasses of db.Error that would not make sense to retry the database operation for as they are not related to database connection issues (e.g. ProgrammingError - see PEP249 for details).

I suggest that we start conservatively and only add more exceptions as necessary. I have pushed bdac27c382341fc4181036806e21425041dcc1e2 which adds support for db.InterfaceError.