[16.0][IMP] database_cleanup: purge orphaned attachments - #3566
Conversation
699d8f0 to
ab70051
Compare
StefanRijnhart
left a comment
StefanRijnhart
left a comment
There was a problem hiding this comment.
Thanks! Judging from the name of the PR and the patterns from this module, I would have expected signalling attachments where the model no longer exists or the res_id is not present as a record. This makes sense as well, but to make it explicit what the issue is with each attachment (and to prepare for extension with other possible error conditions), would you mind adding a reason field to the wizard line model as is done in cleanup.purge.line, with for now a fixed reason like 'File missing in filestore'?
b372147 to
95fb498
Compare
StefanRijnhart
left a comment
StefanRijnhart
left a comment
There was a problem hiding this comment.
Thanks for the updates! Please squash commits.
95fb498 to
e8b0061
Compare
There was a problem hiding this comment.
Thanks, the error message is stored correctly.
Usability wise, I think as a user I would want to see some more information about the attachments, the res_model, res_id and res_name, with or without optional visibility. Is this not something that you want for your own purposes?
Our objective in creating this PR was to clean approximately 15,000 orphaned attachments simultaneously. We didn’t have to inspect every single attachment. |
StefanRijnhart
left a comment
StefanRijnhart
left a comment
There was a problem hiding this comment.
OK, leaving my last comment as a nice to have. Nice work!
| from .common import Common, environment | ||
|
|
||
|
|
||
| # Use post_install to get all models loaded more info: odoo/odoo#13458 |
There was a problem hiding this comment.
I don't think this comment belongs here
|
|
||
| :var orphan: ir.attachment with backing file removed via os.unlink | ||
| :var valid: ir.attachment with file intact | ||
| :return: None |
There was a problem hiding this comment.
remove redundant lines such as ":return: None" to avoid unnecessary noise
e8b0061 to
831b3af
Compare
831b3af to
49ce4fe
Compare
|
|
||
| @tagged("post_install", "-at_install") | ||
| class TestCleanupPurgeLineAttachment(Common): | ||
| def setUp(self): |
There was a problem hiding this comment.
please rewrite it to use setUpClass, since that works faster
|
It appears that the CI failure here is unrelated to this change. Check this comment. |
Add wizard to list ir.attachment records whose store_fname points to a missing file on disk when storage is "file", and allow purging those rows. task 5258
1a94b71 to
c7c414d
Compare
On databases with hundreds of thousands of attachments find() searched on all ir.attachment could raise MemoryError before any file is checked.
4 participants
Add wizard to list ir.attachment records whose store_fname points to a missing file on disk when storage is "file", and allow purging those rows.