flake8
-
reviewboard/attachments/signal_handlers.py (Diff revision 1) Show all issues
Review Request #13142 — Created July 13, 2023 and submitted
Currently, when file attachments get completely deleted from the database we
keep their associated files lying around. There's no good reason to do this,
when the file attachment gets deleted we have no way of accessing these files
anymore.This change deletes associated files from the filesystem when file
attachments get deleted. Any files created by theMimetypeHandler
will be
deleted, and then the main file of the file attachment will be deleted.
.png
file attachments that have thumbnail files,.txt
files.Summary |
---|
Description | From | Last Updated |
---|---|---|
no newline at end of file Column: 46 Error code: W292 |
![]() |
|
Can you add -> None so this will opt into type hints? You'll also need an annotations future import here … |
|
|
Since we don't use this outside of typing, you should import typing.TYPE_CHECKING and then do this import in an if … |
|
|
I think we should override FileAttachment.delete to call delete_associated_files before performing the main deletion functionality. We wouldn't need to worry … |
|
Description: |
|
||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+170) |
Testing Done: |
|
||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
||||||||||||||||||||||||
Branch: |
|
||||||||||||||||||||||||
Diff: |
Revision 3 (+170) |
reviewboard/attachments/apps.py (Diff revision 3) |
---|
Can you add
-> None
so this will opt into type hints?You'll also need an
annotations
future import here and in other new files.
reviewboard/attachments/signal_handlers.py (Diff revision 3) |
---|
Since we don't use this outside of typing, you should import
typing.TYPE_CHECKING
and then do this import in anif TYPE_CHECKING:
block (which is basically treated as a 4th import group).
reviewboard/attachments/signal_handlers.py (Diff revision 3) |
---|
I think we should override
FileAttachment.delete
to calldelete_associated_files
before performing the main deletion functionality.We wouldn't need to worry about the order of signal handling or anything in that case, and can keep all deletion logic in one place.
Summary: |
|
||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+166) |