Code cleanup - #950
Conversation
🤖 Augment PR SummarySummary: This PR continues a code-quality cleanup pass across Ghostwriter. Changes:
Technical Notes: The intent appears to be tightening error handling while keeping runtime behavior the same, with additional debug-level observability where exceptions were previously swallowed. 🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
Pull request overview
This PR continues code-quality cleanup across Ghostwriter’s Django backend by tightening exception handling, replacing silent pass blocks with debug logging, and simplifying tests/assertions for clarity.
Changes:
- Replaced bare
except:clauses with explicit exception types (e.g.,Exception,ValueError,TypeError) in tests, migrations, and runtime code paths. - Replaced several silent exception
passblocks withlogger.debug(...)to preserve observability without changing user-facing behavior. - Minor test cleanups (assertion simplifications, removing unused variables/comments).
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ghostwriter/shepherd/tests/test_models.py | Narrows bare except to except Exception in model URL tests. |
| ghostwriter/shepherd/tasks.py | Replaces silent exception handling with debug logging for missing history / non-JSON API responses. |
| ghostwriter/shepherd/migrations/0037_convert_dns_record_to_json.py | Narrows migration exception handling to explicit exception types. |
| ghostwriter/rolodex/views.py | Adds debug logging when a URL kwarg is not numeric instead of silently ignoring. |
| ghostwriter/rolodex/tests/test_models.py | Narrows bare except to except Exception in model URL tests. |
| ghostwriter/reporting/views2/report.py | Logs WebSocket send failures (gaierror) instead of silently ignoring. |
| ghostwriter/reporting/views2/report_observation_link.py | Logs WebSocket send failures (gaierror) instead of silently ignoring. |
| ghostwriter/reporting/views2/report_finding_link.py | Logs WebSocket send failures (gaierror) instead of silently ignoring. |
| ghostwriter/reporting/tests/test_views.py | Simplifies assertions and removes unused locals in tests. |
| ghostwriter/reporting/tests/test_rich_text_docx.py | Removes commented-out manual inspection code in tests. |
| ghostwriter/reporting/tests/test_models.py | Narrows bare except to except Exception and simplifies assertions/unused locals. |
| ghostwriter/reporting/models.py | Refactors default-weight helper and adds debug logging for missing prior DB entry. |
| ghostwriter/reporting/migrations/0026_convert_linting_status_to_json.py | Narrows migration exception handling to explicit exception types. |
| ghostwriter/oplog/views.py | Replaces silent exception handling with debug logging during export/zip operations. |
| ghostwriter/oplog/tests/test_forms.py | Removes an unused local in a form test. |
| ghostwriter/modules/reportwriter/richtext/ooxml.py | Logs invalid font-size style values instead of silently ignoring. |
| ghostwriter/modules/reportwriter/richtext/docx.py | Logs missing DOCX styles instead of silently ignoring; removes unused locals. |
| ghostwriter/modules/health_utils.py | Narrows bare except to except Exception in cache health checks. |
| ghostwriter/modules/custom_serializers.py | Adds debug logging for a caught KeyError; introduces multiple trivial __init__ overrides. |
| ghostwriter/modules/cloud_monitors.py | Removes redundant ignore_tags initialization in S3 fetcher. |
| ghostwriter/home/templatetags/custom_tags.py | Adds logging and logs parse failures instead of silently ignoring. |
| ghostwriter/api/views.py | Uses json.JSONDecodeError consistently (removing separate import) and logs missing recording replacement case. |
Comments suppressed due to low confidence (6)
ghostwriter/modules/custom_serializers.py:311
- This init override only calls
super().__init__and adds no behavior. It increases noise and may trigger linter warnings (e.g., useless-super-delegation). Remove it unless you plan to add initialization logic.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:352
- This init override only calls
super().__init__and adds no behavior. It increases noise and may trigger linter warnings (e.g., useless-super-delegation). Remove it unless you plan to add initialization logic.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:579
- This init override only calls
super().__init__and adds no behavior. It increases noise and may trigger linter warnings (e.g., useless-super-delegation). Remove it unless you plan to add initialization logic.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:672
- This init override only calls
super().__init__and adds no behavior. It increases noise and may trigger linter warnings (e.g., useless-super-delegation). Remove it unless you plan to add initialization logic.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:781
- This init override only calls
super().__init__and adds no behavior. It increases noise and may trigger linter warnings (e.g., useless-super-delegation). Remove it unless you plan to add initialization logic.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:806
- This init override only calls
super().__init__and adds no behavior. It increases noise and may trigger linter warnings (e.g., useless-super-delegation). Remove it unless you plan to add initialization logic.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (6)
ghostwriter/modules/custom_serializers.py:288
- This
__init__override only delegates tosuper()and does not change serializer behavior. It adds noise and makes future diffs harder to read; consider removing it.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:352
- This
__init__override only delegates tosuper()and does not change serializer behavior. It adds noise and makes future diffs harder to read; consider removing it.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:579
- This
__init__override only delegates tosuper()and does not change serializer behavior. It adds noise and makes future diffs harder to read; consider removing it.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:672
- This
__init__override only delegates tosuper()and does not change serializer behavior. It adds noise and makes future diffs harder to read; consider removing it.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:781
- This
__init__override only delegates tosuper()and does not change serializer behavior. It adds noise and makes future diffs harder to read; consider removing it.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
ghostwriter/modules/custom_serializers.py:806
- This
__init__override only delegates tosuper()and does not change serializer behavior. It adds noise and makes future diffs harder to read; consider removing it.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #950 +/- ##
==========================================
+ Coverage 92.16% 92.18% +0.01%
==========================================
Files 442 443 +1
Lines 33796 33872 +76
==========================================
+ Hits 31149 31224 +75
- Misses 2647 2648 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| type_or_template_id = self.kwargs["type_or_template_id"] | ||
| try: | ||
| type_or_template_id = int(type_or_template_id) | ||
| except ValueError: | ||
| pass | ||
| logger.debug("Report type or template ID is not numeric: %s", type_or_template_id) | ||
|
|
| from datetime import datetime | ||
| import io | ||
| from typing import Any, Iterable | ||
| from typing import Any, Callable, Iterable | ||
| import re | ||
| from venv import logger |
Continuing code quality clean up.