[#39] Hardened certificate admin changelist to not crash on missing physical files #40
Conversation
Codecov Report
@@ Coverage Diff @@
## master #40 +/- ##
==========================================
+ Coverage 46.17% 46.62% +0.44%
==========================================
Files 36 36
Lines 1306 1317 +11
Branches 222 222
==========================================
+ Hits 603 614 +11
Misses 669 669
Partials 34 34
Continue to review full report at Codecov.
|
Bartvaderkin
force-pushed
the
fix/39-certificate-filenotfound-listerror
branch
from
bcf19f4
to
43e8a7a
Compare
Bartvaderkin
force-pushed
the
fix/39-certificate-filenotfound-listerror
branch
from
43e8a7a
to
bb24c9b
Compare
| def expiry_date(self, obj=None): | ||
| # alias model property to catch file not found errors | ||
| try: | ||
| return obj.expiry_date | ||
| except FileNotFoundError: | ||
| return _("file not found") | ||
|
|
||
| def is_valid_key_pair(self, obj=None): | ||
| # alias model method to catch file not found errors | ||
| try: | ||
| return obj.is_valid_key_pair() | ||
| except FileNotFoundError: | ||
| return _("file not found") |
There was a problem hiding this comment.
I'd rather see this handled on the model methods/properties, since calling that code somewhere else than in the admin would still cause crashes.
I'd also like to see a logger.error(...) call if the FileNotFoundError happens, that needs to end up in Sentry imo
There was a problem hiding this comment.
Yes but that is what we want: regular use shouldn't silently proceed when missing missing certificate/key files.
But the admin needs to be able to handle the situation so we can fix it.
|
I'll merge it and we can discuss what should happen at the model layer later |
Fixes #39